[FFmpeg-devel] [PATCH 4/4] lavc/cbs_h2645: fix no slice data trigger the assert.

[mypopy at gmail.com]

2018-05-11 18:10 GMT+08:00 Mark Thompson <sw at jkqxz.net>:
> On 11/05/18 06:11, Jun Zhao wrote:
>> when the NALU data with zero, just give a warning.
>>
>> Fixes ticket #7200
>>
>> Signed-off-by: Jun Zhao <mypopydev at gmail.com>
>> ---
>>  libavcodec/cbs_h2645.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
>> index ab33cdb..08b060c 100644
>> --- a/libavcodec/cbs_h2645.c
>> +++ b/libavcodec/cbs_h2645.c
>> @@ -521,7 +521,11 @@ static int cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
>>          // Remove trailing zeroes.
>>          while (size > 0 && nal->data[size - 1] == 0)
>>              --size;
>> -        av_assert0(size > 0);
>> +        if (!size) {
>> +            av_log(ctx->log_ctx, AV_LOG_WARNING, "No slice data - that was just the header. "
>> +                   "Probably invalid unaligned padding on non-final NAL unit.\n");
>> +            continue;
>> +        }
>>
>>          data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
>>          if (!data)
>>
>
> What do we actually want the result to be here?
>
> On IRC, James suggested:
>
>> diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
>> index dbf2435677..d436d65f48 100644
>> --- a/libavcodec/h2645_parse.c
>> +++ b/libavcodec/h2645_parse.c
>> @@ -371,7 +371,7 @@ int ff_h2645_packet_split(H2645Packet *pkt, const uint8_t *buf, int length,
>>              ret = hevc_parse_nal_header(nal, logctx);
>>          else
>>              ret = h264_parse_nal_header(nal, logctx);
>> -        if (ret <= 0 || nal->size <= 0) {
>> +        if (ret <= 0 || nal->size <= 0 || nal->size_bits <= 0) {
>>              if (ret < 0) {
>>                  av_log(logctx, AV_LOG_ERROR, "Invalid NAL unit %d, skipping.\n",
>>                         nal->type);
>
> which removes it before it gets to the CBS code.
I agree this way, I think we need drop the invalid NAL unit as early
as possible.
>
> Another thing we could do is:
>
>> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
>> index ab33cdb69b..46cd887cdd 100644
>> --- a/libavcodec/cbs_h2645.c
>> +++ b/libavcodec/cbs_h2645.c
>> @@ -519,7 +519,7 @@ static int cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
>>          uint8_t *data;
>>
>>          // Remove trailing zeroes.
>> -        while (size > 0 && nal->data[size - 1] == 0)
>> +        while (size > 1 && nal->data[size - 1] == 0)
>>              --size;
>>          av_assert0(size > 0);
>>
>
> which would make it parse as an empty NAL unit of type 0 (unspecified), and therefore pass through into the output stream in the h264_metadata case.
>
> So, what do you think?  Do you know what made your sample stream?
I got this sample from a live streaming  android phone APPs
>
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org