[04/13] rtpdec_h264: Return immediately on errors in h264_handle_packet_stap_a

Message ID 1424378051-6607-4-git-send-email-martin@martin.st
State Committed
Commit 176903ce833ce7469f411640e9748a0d549b5285
Headers show

Commit Message

Martin Storsjö Feb. 19, 2015, 8:34 p.m.
---
 libavformat/rtpdec_h264.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Luca Barbato Feb. 19, 2015, 9:04 p.m. | #1
On 19/02/15 21:34, Martin Storsjö wrote:
> ---
>  libavformat/rtpdec_h264.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavformat/rtpdec_h264.c b/libavformat/rtpdec_h264.c
> index 8dab0d2..5b87529 100644
> --- a/libavformat/rtpdec_h264.c
> +++ b/libavformat/rtpdec_h264.c
> @@ -208,6 +208,7 @@ static int h264_handle_packet_stap_a(AVFormatContext *ctx, AVPacket *pkt,
>              } else {
>                  av_log(ctx, AV_LOG_ERROR,
>                         "nal size exceeds length: %d %d\n", nal_size, src_len);
> +                return AVERROR_INVALIDDATA;
>              }
>  
>              // eat what we handled
> 

Ok.
Vittorio Giovara Feb. 20, 2015, 3:38 p.m. | #2
On Thu, Feb 19, 2015 at 3:34 PM, Martin Storsjö <martin@martin.st> wrote:
> ---
>  libavformat/rtpdec_h264.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/libavformat/rtpdec_h264.c b/libavformat/rtpdec_h264.c
> index 8dab0d2..5b87529 100644
> --- a/libavformat/rtpdec_h264.c
> +++ b/libavformat/rtpdec_h264.c
> @@ -208,6 +208,7 @@ static int h264_handle_packet_stap_a(AVFormatContext *ctx, AVPacket *pkt,
>              } else {
>                  av_log(ctx, AV_LOG_ERROR,
>                         "nal size exceeds length: %d %d\n", nal_size, src_len);
> +                return AVERROR_INVALIDDATA;
>              }
>
>              // eat what we handled

Do you think you could just add one line in the commit log mentioning
the reason for the change?
Martin Storsjö Feb. 20, 2015, 3:43 p.m. | #3
On Fri, 20 Feb 2015, Vittorio Giovara wrote:

> On Thu, Feb 19, 2015 at 3:34 PM, Martin Storsjö <martin@martin.st> wrote:
>> ---
>>  libavformat/rtpdec_h264.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/libavformat/rtpdec_h264.c b/libavformat/rtpdec_h264.c
>> index 8dab0d2..5b87529 100644
>> --- a/libavformat/rtpdec_h264.c
>> +++ b/libavformat/rtpdec_h264.c
>> @@ -208,6 +208,7 @@ static int h264_handle_packet_stap_a(AVFormatContext *ctx, AVPacket *pkt,
>>              } else {
>>                  av_log(ctx, AV_LOG_ERROR,
>>                         "nal size exceeds length: %d %d\n", nal_size, src_len);
>> +                return AVERROR_INVALIDDATA;
>>              }
>>
>>              // eat what we handled
>
> Do you think you could just add one line in the commit log mentioning
> the reason for the change?

Something like this? "Previously, errors were only logged but the code 
kept on trying, and never actually returning the error as a return value."

// Martin
Vittorio Giovara Feb. 20, 2015, 3:51 p.m. | #4
On Fri, Feb 20, 2015 at 10:43 AM, Martin Storsjö <martin@martin.st> wrote:
> On Fri, 20 Feb 2015, Vittorio Giovara wrote:
>
>> On Thu, Feb 19, 2015 at 3:34 PM, Martin Storsjö <martin@martin.st> wrote:
>>>
>>> ---
>>>  libavformat/rtpdec_h264.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/libavformat/rtpdec_h264.c b/libavformat/rtpdec_h264.c
>>> index 8dab0d2..5b87529 100644
>>> --- a/libavformat/rtpdec_h264.c
>>> +++ b/libavformat/rtpdec_h264.c
>>> @@ -208,6 +208,7 @@ static int h264_handle_packet_stap_a(AVFormatContext
>>> *ctx, AVPacket *pkt,
>>>              } else {
>>>                  av_log(ctx, AV_LOG_ERROR,
>>>                         "nal size exceeds length: %d %d\n", nal_size,
>>> src_len);
>>> +                return AVERROR_INVALIDDATA;
>>>              }
>>>
>>>              // eat what we handled
>>
>>
>> Do you think you could just add one line in the commit log mentioning
>> the reason for the change?
>
>
> Something like this? "Previously, errors were only logged but the code kept
> on trying, and never actually returning the error as a return value."

yeah, looks nice

Patch

diff --git a/libavformat/rtpdec_h264.c b/libavformat/rtpdec_h264.c
index 8dab0d2..5b87529 100644
--- a/libavformat/rtpdec_h264.c
+++ b/libavformat/rtpdec_h264.c
@@ -208,6 +208,7 @@  static int h264_handle_packet_stap_a(AVFormatContext *ctx, AVPacket *pkt,
             } else {
                 av_log(ctx, AV_LOG_ERROR,
                        "nal size exceeds length: %d %d\n", nal_size, src_len);
+                return AVERROR_INVALIDDATA;
             }
 
             // eat what we handled