Re: [FFmpeg-devel] [PATCH] ffprobe/eac3/mlp/dca: add detection of spatial audio extensions

[Marth64 <marth64@proxyid.net>]

Hi Hendrik,

Thank you for taking the time to help me test this. I will verify this as
well on my corpus of 15 or so titles and commit/sign if it passes there
too. Will update within 24.

Respectfully,
Marth64

On Thu, Feb 16, 2023 at 06:37 Hendrik Leppkes <h.leppkes@gmail.com> wrote:

> On Sun, Feb 12, 2023 at 1:53 AM Marth64 <marth64@proxyid.net> wrote:
> > diff --git a/libavcodec/dca_syncwords.h b/libavcodec/dca_syncwords.h
> > index 4d2cd5f56d..200702f89e 100644
> > --- a/libavcodec/dca_syncwords.h
> > +++ b/libavcodec/dca_syncwords.h
> > @@ -33,4 +33,7 @@
> >  #define    DCA_SYNCWORD_SUBSTREAM_CORE       0x02B09261U
> >  #define    DCA_SYNCWORD_REV1AUX              0x9A1105A0U
> >
> > +#define    DCA_SYNCWORD_XLL_X                0x00020008U
> > +#define    DCA_SYNCWORD_XLL_X_IMAX           0x00F14000U
> > +
> >  #endif /* AVCODEC_DCA_SYNCWORDS_H */
> > diff --git a/libavcodec/dca_xll.c b/libavcodec/dca_xll.c
> > index fe2c766d98..efbbae67f8 100644
> > --- a/libavcodec/dca_xll.c
> > +++ b/libavcodec/dca_xll.c
> > @@ -19,6 +19,7 @@
> >   */
> >
> >  #include "libavutil/channel_layout.h"
> > +#include "avcodec.h"
> >  #include "dcadec.h"
> >  #include "dcadata.h"
> >  #include "dcamath.h"
> > @@ -1043,6 +1044,7 @@ static int parse_band_data(DCAXllDecoder *s)
> >  static int parse_frame(DCAXllDecoder *s, const uint8_t *data, int size,
> DCAExssAsset *asset)
> >  {
> >      int ret;
> > +    int extradata_peek_pos;
> >
> >      if ((ret = init_get_bits8(&s->gb, data, size)) < 0)
> >          return ret;
> > @@ -1054,10 +1056,23 @@ static int parse_frame(DCAXllDecoder *s, const
> uint8_t *data, int size, DCAExssA
> >          return ret;
> >      if ((ret = parse_band_data(s)) < 0)
> >          return ret;
> > +
> > +    extradata_peek_pos = (get_bits_count(&s->gb) + 31) & ~31;
> > +    if (s->frame_size * 8 > extradata_peek_pos) {
> > +        unsigned int extradata_syncword = show_bits_long(&s->gb, 32);
> > +
> > +        if (extradata_syncword == DCA_SYNCWORD_XLL_X) {
> > +            s->x_syncword_present = 1;
> > +        } else if (extradata_syncword == DCA_SYNCWORD_XLL_X_IMAX) {
> > +            s->x_imax_syncword_present = 1;
> > +        }
> > +    }
> > +
>
>
> I was testing this, and the DTS detections were not very reliable for
> me. This is what I came up with instead:
>
> #define    DCA_SYNCWORD_XLL_X                0x02000850U
> #define    DCA_SYNCWORD_XLL_X_IMAX           0xF14000D0U
>
>
>     if (s->frame_size * 8 > FFALIGN(get_bits_count(&s->gb), 32)) {
>         unsigned int extradata_syncword;
>
>         // align to dword
>         skip_bits_long(&s->gb, -get_bits_count(&s->gb) & 31);
>
>         // get sync code
>         extradata_syncword = show_bits_long(&s->gb, 32);
>
>         if (extradata_syncword == DCA_SYNCWORD_XLL_X) {
>             s->x_syncword_present = 1;
>         } else if ((extradata_syncword >> 1) ==
> (DCA_SYNCWORD_XLL_X_IMAX >> 1)) {
>             s->x_imax_syncword_present = 1;
>         }
>     }
>
> I don't have many DTS:X discs, but this worked on them, the old code did
> not.
> Aligning to DWORD for a new section is a typical DTS thing to do,
> which then also resulted in the syncwords to shift a bit, and actually
> include more digits.
>
> The IMAX case is a bit weird, there seems to be an extra bit in there
> thats not stable, so shifting it out improves the detection (or it
> could be masked out, but same difference).
>
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@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-request@ffmpeg.org with subject "unsubscribe".