Re: [FFmpeg-devel] [PATCH v22 02/10] avcodec/evc_parser: Added parser implementation for EVC format

[Dawid Kozinski/Multimedia (PLT) /SRPOL/Staff Engineer/Samsung Electronics]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of James
> Almer
> Sent: piątek, 19 maja 2023 17:24
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v22 02/10] avcodec/evc_parser: Added
> parser implementation for EVC format
> 
> On 5/19/2023 7:31 AM, Dawid Kozinski/Multimedia (PLT) /SRPOL/Staff
> Engineer/Samsung Electronics wrote:
> >
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
> >> James Almer
> >> Sent: środa, 10 maja 2023 22:21
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH v22 02/10] avcodec/evc_parser:
> >> Added parser implementation for EVC format
> >>
> >> On 4/27/2023 9:02 AM, Dawid Kozinski wrote:
> >>> - Added constants definitions for EVC parser
> >>> - Provided NAL units parsing following ISO_IEC_23094-1
> >>> - EVC parser registration
> >>>
> >>> Signed-off-by: Dawid Kozinski <d.kozin...@samsung.com>
> >>> ---
> >>>    libavcodec/Makefile     |    1 +
> >>>    libavcodec/evc.h        |  155 ++++
> >>>    libavcodec/evc_parser.c | 1497
> >> +++++++++++++++++++++++++++++++++++++++
> >>>    libavcodec/parsers.c    |    1 +
> >>>    4 files changed, 1654 insertions(+)
> >>>    create mode 100644 libavcodec/evc.h
> >>>    create mode 100644 libavcodec/evc_parser.c
> >>
> >> There seems to have been a regression in this version compared to v20.
> >> Try to compile with the libxevd decoder disabled and open a raw file
> >> (not
> > mp4).
> >>
> >> Whereas with v20 i was getting
> >>
> >>> Input #0, evc, from 'akiyo_cif.evc':
> >>>    Duration: N/A, bitrate: N/A
> >>>    Stream #0:0: Video: evc (Baseline), none, 352x288, 25 fps, 25
> >>> tbr, 1200k tbn
> >>
> >> I'm now getting
> >>
> >>> Input #0, evc, from 'akiyo_cif.evc':
> >>>    Duration: N/A, bitrate: N/A
> >>>    Stream #0:0: Video: evc (Baseline), none, 555902582x0, 25 fps, 25
> >>> tbr, 1200k tbn
> >>
> >> It seems that the problem showed up because you moved the parameter
> >> sets
> > to
> >> stack to skip allocating them, and you no longer check if they exist
> >> or
> > were
> >> parsed by looking at slice_pic_parameter_set_id and such.
> >>
> >> That aside, i looked at the EVC spec and noticed that the "raw"
> >> format in
> > Annex-
> >> B is unlike that of H.26{4,5,6}: There's no start code, and instead
> > there's a NAL
> >> size prefix, which sounds like the isobmff way of encapsulating NALUs.
> >> I also noticed that the syntax for nal_unit() contains an
> >> emulation_prevention_three_byte element, but there's no explanation
> >> for it
> > in
> >> the semantics section. Its existence in H.26* is to prevent parsers
> >> from misinterpreting a sequence of two or more zeroes as a potential
> >> start
> > code, but
> >> that's clearly not the case here, so why is it defined and present at all?
> >>
> >> What this means is that the parser can't be made to assemble an AU.
> >> If you
> > feed
> >> it data starting from the middle of a NAL, it will not be able to
> >> sync to
> > the start
> >> of the next NAL because all it looks for is a four byte size value
> >> and
> > will accept
> >> the very first four bytes its fed as one.
> >> Since i doubt the spec can be changed at this point to amend this
> > oversight, the
> >> AU assembling will have to occur in the evc demuxer, much like we do
> >> with
> > AV1
> >> (both raw obu and Annex-B formats as defined in the corresponding
> >> spec),
> > and
> >> the parser be limited to parse fully assembled NALs with
> > parse_nal_units().
> >
> > According to your last EVC review:
> > We have re-implemented the EVC demuxer to assemble Access Units (AUs)
> > and pass them further, while the EVC parser is limited to parsing
> > complete NAL units provided in consecutive AUs.
> >
> > However, before we create a new patchset, we would like to discuss
> > some things because we believe this solution is not optimal.
> >
> > According to the EVC documentation, "Each access unit contains a set
> > of VCL NAL units that together compose a primary coded picture."
> > This means that to find the boundaries of an AU, we need to identify
> > all the NAL units that contain data for a single encoded picture.
> > In our case, to determine whether the next VCL NAL unit contains data
> > for the same picture as the previous VCL NAL unit or for a different
> > encoded picture, we need information contained in the NAL units. We
> > need to extract certain data from NAL units like SPS and PPS, as well
> > as from the Slice Headers of VCL NAL units.
> > In other words, this implies that parsing NAL units is required for
> > this purpose.
> >
> > It may not be as extensive parsing as in the EVC parser, but still,
> > there is a significant amount of parsing involved, which adds some overhead.
> > Another question is whether the demuxer is the appropriate place for
> > parsing NAL units? I guess it's not.
> >
> > Perhaps it would be better if the demuxer prepared complete NAL units
> > instead of complete AUs. The EVC parser would then receive complete
> > NAL units, and since it already parses them, we have the necessary
> > information for preparing complete AUs at no extra cost.
> >
> > Preparing complete NAL units is definitely simpler than preparing
> > complete AUs. It only requires finding the length of the NAL unit and
> > putting that amount of data from the input into the AVPacket.
> >
> > Please let me know your thoughts on this.
> 
> Yes, i mostly agree with what you said, however the demuxer needs to
> propagate entire AUs, because otherwise in a codec copy scenario, a muxer
> would see packets containing a single NALu, potentially creating non compliant
> files.
> 
> The proper solution for this is to have the demuxer prepare complete NALUs,
> and pass them to a bitstream filter that will assemble an AU, with all this
> happening within the demuxer. The assembled AU is then output by the
> demuxer, and the parser and decoder will only see and handle said assembled
> AU.
> The bitstream filter will reside in libavcodec, which means the sps/pps/slice
> parsing code can be shared between the parser and this bsf, so no code
> duplication in lavc and lavf. This is what we do with
> AV1 for its raw demuxer (See libavformat/av1dec.c and
> libavcodec/av1_frame_merge.c, the latter which uses the CBS framework, but in
> this case you can use the standalone parsing as you already wrote it).
> 
> I also want to apologize for not looking at the spec earlier, to notice the 
> raw
> bitstream was not avparser friendly. It would have saved you a lot of work.

We've uploaded a new patchset.
We know there is a build warning, caused by two unused functions. We'll fix it 
and make a new patchset after your review.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://protect2.fireeye.com/v1/url?k=78d233df-19a99948-78d3b890-
> 74fe4860001d-dccce604599d7a78&q=1&e=9ca258a6-efa2-47ad-94b3-
> 8423deb9d6fa&u=https%3A%2F%2Fffmpeg.org%2Fmailman%2Flistinfo%2Fffmp
> eg-devel
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org
> with subject "unsubscribe".


_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".