On 5/11/2018 7:10 AM, Mark Thompson wrote:
> 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 <mypopy...@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.
> 
> 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?
> 
> - Mark

Taking into account the analysis by mkver in the trac ticket, where he
found out the bitstream contains "00 00 00 01 00 00 00 01" with the
second start code being a real valid NAL, i think this should definitely
be fixed in h2645_parse. No reason to propagate a non existent NAL like
this.
We should either use my fix, or another that actually prevents nal->size
from inexplicably becoming 1 in this scenario.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to