[FFmpeg-devel] avcodec/dca: return standard error codes in avpriv_dca_parse_core_frame_header()

Submitted by James Almer on Oct. 31, 2017, 8:27 p.m.

Details

Message ID 20171031202735.4292-1-jamrial@gmail.com
State New
Headers show

Commit Message

James Almer Oct. 31, 2017, 8:27 p.m.
This prevents making the DCAParseError enum part of the ABI.

Signed-off-by: James Almer <jamrial@gmail.com>
---
Now actually paying attention to -Wempty-body warnings.

 libavcodec/dca.c | 11 ++++++++---
 libavcodec/dca.h | 12 ++++++++++--
 2 files changed, 18 insertions(+), 5 deletions(-)

Comments

Michael Niedermayer Nov. 1, 2017, 2:44 a.m.
On Tue, Oct 31, 2017 at 05:27:35PM -0300, James Almer wrote:
> This prevents making the DCAParseError enum part of the ABI.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> Now actually paying attention to -Wempty-body warnings.
> 
>  libavcodec/dca.c | 11 ++++++++---
>  libavcodec/dca.h | 12 ++++++++++--
>  2 files changed, 18 insertions(+), 5 deletions(-)

works fine here, no more comments from me

thx

[...]
foo86 Nov. 1, 2017, 6:24 p.m.
On Tue, Oct 31, 2017 at 05:27:35PM -0300, James Almer wrote:
> This prevents making the DCAParseError enum part of the ABI.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> Now actually paying attention to -Wempty-body warnings.
> 
>  libavcodec/dca.c | 11 ++++++++---
>  libavcodec/dca.h | 12 ++++++++++--
>  2 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/dca.c b/libavcodec/dca.c
> index 942fe6c3c9..a0729e61ab 100644
> --- a/libavcodec/dca.c
> +++ b/libavcodec/dca.c
> @@ -149,9 +149,14 @@ int ff_dca_parse_core_frame_header(DCACoreFrameHeader *h, GetBitContext *gb)
>  int avpriv_dca_parse_core_frame_header(DCACoreFrameHeader *h, const uint8_t *buf, int size)
>  {
>      GetBitContext gb;
> +    int ret;
>  
> -    if (init_get_bits8(&gb, buf, size) < 0)
> -        return DCA_PARSE_ERROR_INVALIDDATA;
> +    ret = init_get_bits8(&gb, buf, size);
> +    if (ret < 0)
> +        return ret;
>  
> -    return ff_dca_parse_core_frame_header(h, &gb);
> +    if (ff_dca_parse_core_frame_header(h, &gb) < 0)
> +        return AVERROR_INVALIDDATA;
> +
> +    return 0;
>  }
> diff --git a/libavcodec/dca.h b/libavcodec/dca.h
> index c70598af92..b05e5f896e 100644
> --- a/libavcodec/dca.h
> +++ b/libavcodec/dca.h
> @@ -46,7 +46,6 @@ enum DCAParseError {
>      DCA_PARSE_ERROR_RESERVED_BIT    = -7,
>      DCA_PARSE_ERROR_LFE_FLAG        = -8,
>      DCA_PARSE_ERROR_PCM_RES         = -9,
> -    DCA_PARSE_ERROR_INVALIDDATA     = -10,
>  };
>  
>  typedef struct DCACoreFrameHeader {
> @@ -211,10 +210,19 @@ int avpriv_dca_convert_bitstream(const uint8_t *src, int src_size, uint8_t *dst,
>  
>  /**
>   * Parse and validate core frame header
> - * @return 0 on success, negative DCA_PARSE_ERROR_ code on failure
> + * @param[out] h    Pointer to struct where header info is written.
> + * @param[in]  buf  Pointer to the data buffer
> + * @param[in]  size Size of the data buffer
> + * @return 0 on success, negative AVERROR code on failure
>   */
>  int avpriv_dca_parse_core_frame_header(DCACoreFrameHeader *h, const uint8_t *buf, int size);
>  
> +/**
> + * Parse and validate core frame header
> + * @param[out] h   Pointer to struct where header info is written.
> + * @param[in]  gbc BitContext containing the first 54 bits of the frame.

Should be "the first 120 bits".

> + * @return 0 on success, negative DCA_PARSE_ERROR_ code on failure
> + */
>  int ff_dca_parse_core_frame_header(DCACoreFrameHeader *h, GetBitContext *gb);
>  
>  #endif /* AVCODEC_DCA_H */

Patch otherwise OK.
James Almer Nov. 1, 2017, 9:37 p.m.
On 11/1/2017 3:24 PM, foo86 wrote:
> On Tue, Oct 31, 2017 at 05:27:35PM -0300, James Almer wrote:
>> This prevents making the DCAParseError enum part of the ABI.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> Now actually paying attention to -Wempty-body warnings.
>>
>>  libavcodec/dca.c | 11 ++++++++---
>>  libavcodec/dca.h | 12 ++++++++++--
>>  2 files changed, 18 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavcodec/dca.c b/libavcodec/dca.c
>> index 942fe6c3c9..a0729e61ab 100644
>> --- a/libavcodec/dca.c
>> +++ b/libavcodec/dca.c
>> @@ -149,9 +149,14 @@ int ff_dca_parse_core_frame_header(DCACoreFrameHeader *h, GetBitContext *gb)
>>  int avpriv_dca_parse_core_frame_header(DCACoreFrameHeader *h, const uint8_t *buf, int size)
>>  {
>>      GetBitContext gb;
>> +    int ret;
>>  
>> -    if (init_get_bits8(&gb, buf, size) < 0)
>> -        return DCA_PARSE_ERROR_INVALIDDATA;
>> +    ret = init_get_bits8(&gb, buf, size);
>> +    if (ret < 0)
>> +        return ret;
>>  
>> -    return ff_dca_parse_core_frame_header(h, &gb);
>> +    if (ff_dca_parse_core_frame_header(h, &gb) < 0)
>> +        return AVERROR_INVALIDDATA;
>> +
>> +    return 0;
>>  }
>> diff --git a/libavcodec/dca.h b/libavcodec/dca.h
>> index c70598af92..b05e5f896e 100644
>> --- a/libavcodec/dca.h
>> +++ b/libavcodec/dca.h
>> @@ -46,7 +46,6 @@ enum DCAParseError {
>>      DCA_PARSE_ERROR_RESERVED_BIT    = -7,
>>      DCA_PARSE_ERROR_LFE_FLAG        = -8,
>>      DCA_PARSE_ERROR_PCM_RES         = -9,
>> -    DCA_PARSE_ERROR_INVALIDDATA     = -10,
>>  };
>>  
>>  typedef struct DCACoreFrameHeader {
>> @@ -211,10 +210,19 @@ int avpriv_dca_convert_bitstream(const uint8_t *src, int src_size, uint8_t *dst,
>>  
>>  /**
>>   * Parse and validate core frame header
>> - * @return 0 on success, negative DCA_PARSE_ERROR_ code on failure
>> + * @param[out] h    Pointer to struct where header info is written.
>> + * @param[in]  buf  Pointer to the data buffer
>> + * @param[in]  size Size of the data buffer
>> + * @return 0 on success, negative AVERROR code on failure
>>   */
>>  int avpriv_dca_parse_core_frame_header(DCACoreFrameHeader *h, const uint8_t *buf, int size);
>>  
>> +/**
>> + * Parse and validate core frame header
>> + * @param[out] h   Pointer to struct where header info is written.
>> + * @param[in]  gbc BitContext containing the first 54 bits of the frame.
> 
> Should be "the first 120 bits".

Whoops, copy paste mistake from ac3_parser doxy. Changed to 120.

> 
>> + * @return 0 on success, negative DCA_PARSE_ERROR_ code on failure
>> + */
>>  int ff_dca_parse_core_frame_header(DCACoreFrameHeader *h, GetBitContext *gb);
>>  
>>  #endif /* AVCODEC_DCA_H */
> 
> Patch otherwise OK.

Pushed, thanks!

Patch hide | download patch | download mbox

diff --git a/libavcodec/dca.c b/libavcodec/dca.c
index 942fe6c3c9..a0729e61ab 100644
--- a/libavcodec/dca.c
+++ b/libavcodec/dca.c
@@ -149,9 +149,14 @@  int ff_dca_parse_core_frame_header(DCACoreFrameHeader *h, GetBitContext *gb)
 int avpriv_dca_parse_core_frame_header(DCACoreFrameHeader *h, const uint8_t *buf, int size)
 {
     GetBitContext gb;
+    int ret;
 
-    if (init_get_bits8(&gb, buf, size) < 0)
-        return DCA_PARSE_ERROR_INVALIDDATA;
+    ret = init_get_bits8(&gb, buf, size);
+    if (ret < 0)
+        return ret;
 
-    return ff_dca_parse_core_frame_header(h, &gb);
+    if (ff_dca_parse_core_frame_header(h, &gb) < 0)
+        return AVERROR_INVALIDDATA;
+
+    return 0;
 }
diff --git a/libavcodec/dca.h b/libavcodec/dca.h
index c70598af92..b05e5f896e 100644
--- a/libavcodec/dca.h
+++ b/libavcodec/dca.h
@@ -46,7 +46,6 @@  enum DCAParseError {
     DCA_PARSE_ERROR_RESERVED_BIT    = -7,
     DCA_PARSE_ERROR_LFE_FLAG        = -8,
     DCA_PARSE_ERROR_PCM_RES         = -9,
-    DCA_PARSE_ERROR_INVALIDDATA     = -10,
 };
 
 typedef struct DCACoreFrameHeader {
@@ -211,10 +210,19 @@  int avpriv_dca_convert_bitstream(const uint8_t *src, int src_size, uint8_t *dst,
 
 /**
  * Parse and validate core frame header
- * @return 0 on success, negative DCA_PARSE_ERROR_ code on failure
+ * @param[out] h    Pointer to struct where header info is written.
+ * @param[in]  buf  Pointer to the data buffer
+ * @param[in]  size Size of the data buffer
+ * @return 0 on success, negative AVERROR code on failure
  */
 int avpriv_dca_parse_core_frame_header(DCACoreFrameHeader *h, const uint8_t *buf, int size);
 
+/**
+ * Parse and validate core frame header
+ * @param[out] h   Pointer to struct where header info is written.
+ * @param[in]  gbc BitContext containing the first 54 bits of the frame.
+ * @return 0 on success, negative DCA_PARSE_ERROR_ code on failure
+ */
 int ff_dca_parse_core_frame_header(DCACoreFrameHeader *h, GetBitContext *gb);
 
 #endif /* AVCODEC_DCA_H */