[FFmpeg-devel] [PATCH 1/1] avformat/matroska: fully parse stsd atom in v_quicktime tracks

[Stanislav Ionascu]

On Tue, Aug 13, 2019 at 10:22 PM Andreas Rheinhardt
<andreas.rheinhardt at gmail.com> wrote:
>
> Stanislav Ionascu:
> > Per matroska spec, v_quicktime contains the complete stsd atom, after
> > the mandatory size + fourcc. By properly parsing the hvcc sub-atoms of
> > the track, it becomes possible to demux/decode mp4/mov tracks stored as is
> > in matroska containers.
> >
> > Also dvh1 in stsd in matroska is more likely hevc codec than dv.
> >
> > Signed-off-by: Stanislav Ionascu <stanislav.ionascu at gmail.com>
> > ---
> >  libavformat/matroskadec.c | 51 +++++++++++++++++++++++++++------------
> >  1 file changed, 36 insertions(+), 15 deletions(-)
> >
> > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> > index 4e20f15792..88bc89c545 100644
> > --- a/libavformat/matroskadec.c
> > +++ b/libavformat/matroskadec.c
> > @@ -2473,25 +2473,46 @@ static int matroska_parse_tracks(AVFormatContext *s)
> >          } else if (!strcmp(track->codec_id, "V_QUICKTIME") &&
> >                     (track->codec_priv.size >= 21)          &&
> >                     (track->codec_priv.data)) {
> > +            MOVStreamContext *msc;
> > +            MOVContext *mc = NULL;
> > +            AVIOContext *stsd_ctx = NULL;
> > +            void *priv_data;
> > +            int nb_streams;
> >              int ret = get_qt_codec(track, &fourcc, &codec_id);
> >              if (ret < 0)
> >                  return ret;
> > -            if (codec_id == AV_CODEC_ID_NONE && AV_RL32(track->codec_priv.data+4) == AV_RL32("SMI ")) {
> > -                fourcc = MKTAG('S','V','Q','3');
> > -                codec_id = ff_codec_get_id(ff_codec_movvideo_tags, fourcc);
> > +            av_log(matroska->ctx, AV_LOG_TRACE,
> > +                   "FourCC found %s.\n", av_fourcc2str(fourcc));
> > +            priv_data = st->priv_data;
> > +            nb_streams = s->nb_streams;
> > +            mc = av_mallocz(sizeof(*mc));
> > +            if (!mc)
> > +                return AVERROR(ENOMEM);
> > +            stsd_ctx = avio_alloc_context(track->codec_priv.data,
> > +                                track->codec_priv.size,
> > +                                0, NULL, NULL, NULL, NULL);
> > +            if (!stsd_ctx)
> > +                return AVERROR(ENOMEM);
> I haven't looked at this patch deeply yet, but it seems to me that you
> should rather use ffio_init_context like it is done in the code that
> you intend to delete. That saves allocating and freeing stsd_ctx. You
> can even reuse the AVIOContext b that already exists on the stack.

Done.

> > +            mc->fc = s;
> > +            st->priv_data = msc = av_mallocz(sizeof(MOVStreamContext));
> > +            if (!msc) {
> > +                av_free(mc);
> > +                st->priv_data = priv_data;
> > +                return AVERROR(ENOMEM);
> >              }
> > -            if (codec_id == AV_CODEC_ID_NONE)
> > -                av_log(matroska->ctx, AV_LOG_ERROR,
> > -                       "mov FourCC not found %s.\n", av_fourcc2str(fourcc));
> > -            if (track->codec_priv.size >= 86) {
> > -                bit_depth = AV_RB16(track->codec_priv.data + 82);
> > -                ffio_init_context(&b, track->codec_priv.data,
> > -                                  track->codec_priv.size,
> > -                                  0, NULL, NULL, NULL, NULL);
> > -                if (ff_get_qtpalette(codec_id, &b, track->palette)) {
> > -                    bit_depth &= 0x1F;
> > -                    track->has_palette = 1;
> Why are you removing this code? What about tracks that ought to have a
> palette?

The palette parsing is done in the stsd parser, but it still has to be
copied back into the track,
which is done in the later patch.

> > -                }
> > +            /* ff_mov_read_stsd_entries updates stream s->nb_streams-1,
> > +             * so set it temporarily to indicate which stream to update. */
> > +            s->nb_streams = st->index + 1;
> > +            ff_mov_read_stsd_entries(mc, stsd_ctx, 1);
> > +            av_free(msc);
> > +            av_free(mc);
> > +            avio_context_free(&stsd_ctx);
> > +            st->priv_data = priv_data;
> > +            s->nb_streams = nb_streams;
> > +
> > +            // dvh1 in mkv is likely HEVC
> > +            if (st->codecpar->codec_tag == MKTAG('d','v','h','1')) {
> > +                codec_id = AV_CODEC_ID_HEVC;
> >              }
> >          } else if (codec_id == AV_CODEC_ID_PCM_S16BE) {
> >              switch (track->audio.bitdepth) {
> >
>
> Also, your patch should refer to the exact component that is about to
> be changed: avformat/matroskadec.

Done.

>
> - Andreas
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-avformat-matroskadec-properly-parse-stsd-in-v_quickt.patch
Type: application/octet-stream
Size: 4132 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190814/d8cf2d71/attachment.obj>