Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NdkMediaCodec.h: Issue with off_t in signature and use of __USE_FILE_OFFSET64 #459

Closed
koying opened this issue Jul 17, 2017 · 6 comments
Closed
Assignees
Labels
Milestone

Comments

@koying
Copy link

koying commented Jul 17, 2017

Description

NdkMediaCodec.h defines:

media_status_t AMediaCodec_queueInputBuffer(AMediaCodec*,
        size_t idx, **off_t** offset, size_t size, uint64_t time, uint32_t flags);
media_status_t AMediaCodec_queueSecureInputBuffer(AMediaCodec*,
        size_t idx, **off_t** offset, AMediaCodecCryptoInfo*, uint64_t time, uint32_t flags);

When arm32 is built with __USE_FILE_OFFSET64, the off_t's above are defined as off64_t, leading to a non functional MediaCodec (no crashes, though).

Manually changing the off_t to size_tworkarounds the issue (haven't tried arm64, yet, but it should work, too).

Environment Details

Not all of these will be relevant to every bug, but please provide as much
information as you can.

  • NDK Version:15.1.4119039, unified headers
  • Build sytem: Standalone
  • Host OS: Linux Mint x86_64
  • Compiler: GCC
  • ABI: arm32
  • STL: gnu
  • NDK API level: 21
  • Device API level: 24
@DanAlbert DanAlbert self-assigned this Jul 17, 2017
@DanAlbert
Copy link
Member

Good timing, was just getting to cherry-picking _FILE_OFFSET_BITS=64 fixes into r15c. I'll include a fix for this as well.

I grepped the sysroot for any other cases of this outside libc, looks like we have one in asset_manager.h too.

@DanAlbert
Copy link
Member

@koying
Copy link
Author

koying commented Jul 18, 2017

Thanks already.

Am I correct reading that the chosen solution is to enforce undef'ing __USE_FILE_OFFSET64?
Commented on Gerrit.

Regarding the asset manager part, do you confirm the 64 bits version are available on arm32 as well?
Otherwise, that probably won't work...

@enh
Copy link
Contributor

enh commented Jul 18, 2017

no, asset_manager.h actually has both off_t and off64_t variants, so we can make _FILE_OFFSET_BITS=64 do the right thing there. for NdkMediaCodec.h, we're suggesting doing the same as we do for the bionic headers, where you get everything you can have (which in this case means you lose the off_t functions, because there still aren't off64_t variants).

@koying
Copy link
Author

koying commented Jul 18, 2017

Ok, but it makes little sense to remove just those 2 functions, though, as it makes the api unusable.
Those mandatory functions being "hidden" will probably scratch more than a few people heads, so I'd suggest to go the #errorpath altogether, whith an explicit message.

Now, there is still the size_tpath ;)

@enh
Copy link
Contributor

enh commented Jul 18, 2017

yeah, that's why we need to chase down the API owner. if they are mandatory then, yes, #error if _FILE_OFFSET_BITS=64 is defined for LP32 probably makes more sense (and is certainly closer to previous behavior).

hubot pushed a commit to aosp-mirror/platform_bionic that referenced this issue Jul 19, 2017
Convenience macro for __RENAME if __USE_FILE_OFFSET64 mode. Lets us
avoid duplicating all the doxygen comments in frameworks headers.

Test: make checkbuild
Bug: android/ndk#459
Change-Id: Ica44f22b2f1596e484694006c0926d94d16187b5
@DanAlbert DanAlbert added this to the r15c milestone Jul 20, 2017
vlc-mirrorer pushed a commit to videolan/vlc that referenced this issue Oct 24, 2018
vlc-mirrorer pushed a commit to videolan/vlc-3.0 that referenced this issue Oct 24, 2018
cf. android/ndk#459

(cherry picked from commit c454d1737f1b4bf2ba10c1eb3cbdbfc8b37d512b)
Signed-off-by: Thomas Guillem <thomas@gllm.fr>
NotesOfReality pushed a commit to NotesOfReality/android_bionic---DU that referenced this issue Mar 9, 2019
Convenience macro for __RENAME if __USE_FILE_OFFSET64 mode. Lets us
avoid duplicating all the doxygen comments in frameworks headers.

Test: make checkbuild
Bug: android/ndk#459
Change-Id: Ica44f22b2f1596e484694006c0926d94d16187b5
littleGnAl pushed a commit to littleGnAl/mirror_platform_frameworks_av that referenced this issue Feb 9, 2024
Test: make checkbuild
Bug: android/ndk#459
Change-Id: I72528ca13d1f82debec977efbab62e638b000dfc
littleGnAl pushed a commit to littleGnAl/mirror_platform_frameworks_av that referenced this issue Feb 9, 2024
Test: make checkbuild
Bug: android/ndk#459
Change-Id: I72528ca13d1f82debec977efbab62e638b000dfc
littleGnAl pushed a commit to littleGnAl/mirror_platform_frameworks_av that referenced this issue Feb 9, 2024
Test: make checkbuild
Bug: android/ndk#459
Change-Id: I72528ca13d1f82debec977efbab62e638b000dfc
h7su pushed a commit to h7su/android_platform_frameworks_native that referenced this issue Apr 22, 2024
Rename off_t based functions to their 64-bit equivalents if we're in
LP32 with a 64-bit off_t.

We can safely do this for all supported API levels now since API 13 is
no longer supported. Clean up all the guards for that API level while
I'm here.

Test: make checkbuild
Bug: android/ndk#459
Change-Id: I7032e94c7288f87b4895411079216f0ac2cab6bc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants