On 11/05/18 16:38, James Almer wrote: > 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.
I was applying the standard precisely, which I think ends up with the interpretation: 00 00 00 01 09 f0 00 00 00 01 00 00 00 01 41 e2 02 56 | | | | | | | | | | ^^ zero_byte ^^^^^^^^ start code ^^^^^^^^ start code ^^ NAL unit header ^^ NAL unit header ^^^^^^^^ start code ^^ NAL unit content (AUD) ^^ NAL unit header ^^ trailing zeroes ^^^... NAL unit content (slice) The middle NAL unit has type 0 (unspecified application use) and no content. I admit that's probably not what was intended here, but currently we do preserve unspecified NAL units and it's not clear that type 0 should necessarily be treated differently to 24-31. This would be a pretty absurd use, though, so I don't think it really matters. Given that, I'm fine with any of the possible answers above. - Mark (Another data point: the reference decoder segfaults when given the sample stream.) _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel