|
| 1 | +From a5c1d4f8d6d59ec0960b2c13b76431bbbbc61b26 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Lajos Molnar <lajos@google.com> |
| 3 | +Date: Wed, 26 Feb 2025 18:16:50 -0800 |
| 4 | +Subject: [PATCH] mediandk: clean up AMediaCodec_getInput/OutputBuffer |
| 5 | + semantics |
| 6 | + |
| 7 | +AMediaCodec_getInputBuffer erroneously considered the |
| 8 | +offset, which could result in a smaller input buffer being |
| 9 | +returned and client confusion. In practice, input |
| 10 | +buffer offset is rarely used. |
| 11 | + |
| 12 | +Similarly, the buffer size returned from |
| 13 | +AMediaCodec_getOutputBuffer included padding after the |
| 14 | +output buffer leading to potential confusion. |
| 15 | +Now the size returned from both AMediaCodec_getOutputBuffer |
| 16 | +and AMediaCodec_dequeueOutputBuffer is correct. |
| 17 | + |
| 18 | +Also removed mentions of non-existing NDK methods |
| 19 | +AMediaCodec_getOutputBuffers and AMediaCodec_getInputBuffers. |
| 20 | + |
| 21 | +Bug: 301470262 |
| 22 | +Flag: EXEMPT bugfix |
| 23 | +(cherry picked from commit d69fe7b73a0ed14c2b5bc237f1a42314140c9458) |
| 24 | +Merged-in: I6bbd9b85023aef56a608362afde0662d3df7284a |
| 25 | +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:20cca3672f4fbcef3e8dd0cc1a46f585a576ab3c) |
| 26 | +Merged-In: I6bbd9b85023aef56a608362afde0662d3df7284a |
| 27 | +Change-Id: I6bbd9b85023aef56a608362afde0662d3df7284a |
| 28 | +--- |
| 29 | + media/ndk/NdkMediaCodec.cpp | 21 ++++++++++++++++----- |
| 30 | + media/ndk/include/media/NdkMediaCodec.h | 15 +++++++++++++-- |
| 31 | + 2 files changed, 29 insertions(+), 7 deletions(-) |
| 32 | + |
| 33 | +diff --git a/media/ndk/NdkMediaCodec.cpp b/media/ndk/NdkMediaCodec.cpp |
| 34 | +index b230df5179..240848096a 100644 |
| 35 | +--- a/media/ndk/NdkMediaCodec.cpp |
| 36 | ++++ b/media/ndk/NdkMediaCodec.cpp |
| 37 | +@@ -672,7 +672,13 @@ uint8_t* AMediaCodec_getInputBuffer(AMediaCodec *mData, size_t idx, size_t *out_ |
| 38 | + if (out_size != NULL) { |
| 39 | + *out_size = abuf->capacity(); |
| 40 | + } |
| 41 | +- return abuf->data(); |
| 42 | ++ |
| 43 | ++ // When an input buffer is provided to the application, it is essentially |
| 44 | ++ // empty. Ignore its offset as we will set it upon queueInputBuffer. |
| 45 | ++ // This actually works as expected as we do not provide visibility of |
| 46 | ++ // a potential internal offset to the client, so it is equivalent to |
| 47 | ++ // setting the offset to 0 prior to returning the buffer to the client. |
| 48 | ++ return abuf->base(); |
| 49 | + } |
| 50 | + |
| 51 | + android::Vector<android::sp<android::MediaCodecBuffer> > abufs; |
| 52 | +@@ -689,7 +695,7 @@ uint8_t* AMediaCodec_getInputBuffer(AMediaCodec *mData, size_t idx, size_t *out_ |
| 53 | + if (out_size != NULL) { |
| 54 | + *out_size = abufs[idx]->capacity(); |
| 55 | + } |
| 56 | +- return abufs[idx]->data(); |
| 57 | ++ return abufs[idx]->base(); |
| 58 | + } |
| 59 | + ALOGE("couldn't get input buffers"); |
| 60 | + return NULL; |
| 61 | +@@ -704,8 +710,12 @@ uint8_t* AMediaCodec_getOutputBuffer(AMediaCodec *mData, size_t idx, size_t *out |
| 62 | + return NULL; |
| 63 | + } |
| 64 | + |
| 65 | ++ // Note that we do not provide visibility of the internal offset to the |
| 66 | ++ // client, but it also does not make sense to provide visibility of the |
| 67 | ++ // buffer capacity vs the actual size. |
| 68 | ++ |
| 69 | + if (out_size != NULL) { |
| 70 | +- *out_size = abuf->capacity(); |
| 71 | ++ *out_size = abuf->size(); |
| 72 | + } |
| 73 | + return abuf->data(); |
| 74 | + } |
| 75 | +@@ -718,7 +728,7 @@ uint8_t* AMediaCodec_getOutputBuffer(AMediaCodec *mData, size_t idx, size_t *out |
| 76 | + return NULL; |
| 77 | + } |
| 78 | + if (out_size != NULL) { |
| 79 | +- *out_size = abufs[idx]->capacity(); |
| 80 | ++ *out_size = abufs[idx]->size(); |
| 81 | + } |
| 82 | + return abufs[idx]->data(); |
| 83 | + } |
| 84 | +@@ -748,7 +758,8 @@ ssize_t AMediaCodec_dequeueOutputBuffer(AMediaCodec *mData, |
| 85 | + requestActivityNotification(mData); |
| 86 | + switch (ret) { |
| 87 | + case OK: |
| 88 | +- info->offset = offset; |
| 89 | ++ // the output buffer address is already offset in AMediaCodec_getOutputBuffer() |
| 90 | ++ info->offset = 0; |
| 91 | + info->size = size; |
| 92 | + info->flags = flags; |
| 93 | + info->presentationTimeUs = presentationTimeUs; |
| 94 | +diff --git a/media/ndk/include/media/NdkMediaCodec.h b/media/ndk/include/media/NdkMediaCodec.h |
| 95 | +index 598beb709d..223d2f890b 100644 |
| 96 | +--- a/media/ndk/include/media/NdkMediaCodec.h |
| 97 | ++++ b/media/ndk/include/media/NdkMediaCodec.h |
| 98 | +@@ -251,6 +251,11 @@ uint8_t* AMediaCodec_getInputBuffer(AMediaCodec*, size_t idx, size_t *out_size) |
| 99 | + * dequeueOutputBuffer, and not yet queued. |
| 100 | + * |
| 101 | + * Available since API level 21. |
| 102 | ++ * <p> |
| 103 | ++ * At or before API level 35, the out_size returned was invalid, and instead the |
| 104 | ++ * size returned in the AMediaCodecBufferInfo struct from |
| 105 | ++ * AMediaCodec_dequeueOutputBuffer() should be used. After API |
| 106 | ++ * level 35, this API returns the correct output buffer size as well. |
| 107 | + */ |
| 108 | + uint8_t* AMediaCodec_getOutputBuffer(AMediaCodec*, size_t idx, size_t *out_size) __INTRODUCED_IN(21); |
| 109 | + |
| 110 | +@@ -309,9 +314,16 @@ media_status_t AMediaCodec_queueSecureInputBuffer(AMediaCodec*, size_t idx, |
| 111 | + #undef _off_t_compat |
| 112 | + |
| 113 | + /** |
| 114 | +- * Get the index of the next available buffer of processed data. |
| 115 | ++ * Get the index of the next available buffer of processed data along with the |
| 116 | ++ * metadata associated with it. |
| 117 | + * |
| 118 | + * Available since API level 21. |
| 119 | ++ * <p> |
| 120 | ++ * At or before API level 35, the offset in the AMediaCodecBufferInfo struct |
| 121 | ++ * was invalid and should be ignored; however, at the same time |
| 122 | ++ * the buffer size could only be obtained from this struct. After API |
| 123 | ++ * level 35, the offset returned in the struct is always set to 0, and the |
| 124 | ++ * buffer size can also be obtained from the AMediaCodec_getOutputBuffer() call. |
| 125 | + */ |
| 126 | + ssize_t AMediaCodec_dequeueOutputBuffer(AMediaCodec*, AMediaCodecBufferInfo *info, |
| 127 | + int64_t timeoutUs) __INTRODUCED_IN(21); |
| 128 | +@@ -468,7 +480,6 @@ void AMediaCodec_releaseName(AMediaCodec*, char* name) __INTRODUCED_IN(28); |
| 129 | + /** |
| 130 | + * Set an asynchronous callback for actionable AMediaCodec events. |
| 131 | + * When asynchronous callback is enabled, it is an error for the client to call |
| 132 | +- * AMediaCodec_getInputBuffers(), AMediaCodec_getOutputBuffers(), |
| 133 | + * AMediaCodec_dequeueInputBuffer() or AMediaCodec_dequeueOutputBuffer(). |
| 134 | + * |
| 135 | + * AMediaCodec_flush() behaves differently in asynchronous mode. |
| 136 | +-- |
| 137 | +2.49.0.1077.gc0e912fd4c-goog |
| 138 | + |
0 commit comments