[FFmpeg-devel] [PATCH v6 1/3] hevc_mp4toannexb: Insert correct parameter sets before IRAP

[Michael Niedermayer]

On Sun, Nov 24, 2019 at 11:29:18AM -0500, Andriy Gelman wrote:
> On Sun, 24. Nov 00:02, Michael Niedermayer wrote:
> > On Tue, Oct 15, 2019 at 10:50:39PM -0400, Andriy Gelman wrote:
> > > From: Andriy Gelman <andriy.gelman at gmail.com>
> > > 
> > > Fixes #7799
> > > 
> > > Currently, the mp4toannexb filter always inserts the same extradata at
> > > the start of the first IRAP unit. As in ticket #7799, this can lead to
> > > decoding errors if modified parameter sets are signalled in-band.
> > > 
> > > Decoding errors/visual artifacts are also present in the following fate-suite/hevc-conformance datasets for hevc->mp4->hevc conversion:
> > >  -RAP_B_Bossen_1.bit
> > >  -RPS_C_ericsson_5.bit
> > >  -SLIST_A_Sony_4.bit
> > >  -SLIST_B_Sony_8.bit
> > >  -SLIST_C_Sony_3.bit
> > >  -SLIST_D_Sony_9.bit
> > >  -TSKIP_A_MS_2.bit
> > > 
> > > This commit solves these errors by keeping track of VPS/SPS/PPS parameter sets
> > > during the conversion. The correct combination is inserted at the start
> > > of the first IRAP. SEIs from extradata are inserted before each IRAP.
> > > 
> > > This commit also makes an update to the hevc-bsf-mp4toannexb fate test
> > > since the result before this patch contained duplicate parameter sets
> > > in-band.
> > > ---
> > >  libavcodec/hevc_mp4toannexb_bsf.c | 503 ++++++++++++++++++++++++++++--
> > >  tests/fate/hevc.mak               |   2 +-
> > >  2 files changed, 472 insertions(+), 33 deletions(-)
[...]
> > 
> > [...]
> > > +/*
> > > + * Function converts mp4 access unit into annexb
> > > + * Output packet structure
> > > + * VPS, SPS, PPS, [SEI(from extradata)], [SEI_PREFIX(from access unit)], IRAP, [SEI_SUFFIX]
> > > + * or
> > > + * [SEI_PREFIX (from access unit)], [PPS (if not already signalled)], VCL(non-irap), [SEI_SUFFIX]
> > > + */
> > >  static int hevc_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *out)
> > >  {
> > >      HEVCBSFContext *s = ctx->priv_data;
> > >      AVPacket *in;
> > > -    GetByteContext gb;
> > > -
> > > -    int got_irap = 0;
> > > -    int i, ret = 0;
> > > +    H2645Packet pkt;
> > > +    int i, j, prev_size, ret;
> > > +    int irap_done;
> > >  
> > >      ret = ff_bsf_get_packet(ctx, &in);
> > >      if (ret < 0)
> > >          return ret;
> > >  
> > > +    /* output the annexb nalu if extradata is not parsed*/
> > >      if (!s->extradata_parsed) {
> > >          av_packet_move_ref(out, in);
> > >          av_packet_free(&in);
> > >          return 0;
> > >      }
> > >  
> > 
> > > -    bytestream2_init(&gb, in->data, in->size);
> > 
> > Is this really better without using an existing bytestream* API ?
> 
> If I use the API, then I'd have to call bytestraem2_init for each nal unit. I
> also don't use the bytestream2_get_byte function. It seemed simpler to use the WRITE_NAL macro.
> 
> But maybe I've misunderstood your question.

i had nothing really specific in mind, just the feeling that using none of
the existing APIs there is a bit odd.

but more specifically, what about the write side ?

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct answer.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20191125/4986d9dc/attachment.sig>