[FFmpeg-devel] [PATCH 1/1] avformat/matroska: fully parse stsd atom in v_quicktime tracks
Stanislav Ionascu
stanislav.ionascu at gmail.com
Sat Aug 17 13:38:15 EEST 2019
Hi!
On Sat, Aug 17, 2019 at 12:53 AM Michael Niedermayer
<michael at niedermayer.cc> wrote:
>
> On Thu, Aug 15, 2019 at 07:59:28AM +0200, Stanislav Ionascu wrote:
> > Hi,
> >
> > On Wed, Aug 14, 2019 at 11:50 PM Michael Niedermayer
> > <michael at niedermayer.cc> wrote:
> > >
> > > On Wed, Aug 14, 2019 at 08:44:11PM +0200, Stanislav Ionascu wrote:
> > > > 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".
> > >
> > > > matroskadec.c | 61 ++++++++++++++++++++++++++++++++++++++++------------------
> > > > 1 file changed, 43 insertions(+), 18 deletions(-)
> > > > 5dbf0d633a4567ad24eb07356d5de3c4a67b19e0 0001-avformat-matroskadec-properly-parse-stsd-in-v_quickt.patch
> > > > From 18d1e18e3c72f70c2d9aaf2e8d9270a28b4895a2 Mon Sep 17 00:00:00 2001
> > > > From: Stanislav Ionascu <stanislav.ionascu at gmail.com>
> > > > Date: Sun, 11 Aug 2019 21:10:30 +0200
> > > > Subject: [PATCH] avformat/matroskadec: properly parse stsd in v_quicktime
> > > >
> > > > 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.
> > > > QuickTime palette parsing is reused from the stsd parser results.
> > > >
> > > > Signed-off-by: Stanislav Ionascu <stanislav.ionascu at gmail.com>
> > > > ---
> > > > libavformat/matroskadec.c | 61 +++++++++++++++++++++++++++------------
> > > > 1 file changed, 43 insertions(+), 18 deletions(-)
> > >
> > > breaks ./ffmpeg -i Earth\ Spin\ 1-bit\ qtrle.mkv -f null -
> > > link to the file is in bf42a7ef6d073221915dcc042c080374045ab245 (and still works as of this writing)
> > >
> >
> > My bad. Should've stored temporarily the codec_id from codecpar, as it
> > will write it back.
> >
> > Thanks!
> >
> >
> >
> > > thx
> > >
> > > [...]
> > >
> > > --
> > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> > >
> > > The educated differ from the uneducated as much as the living from the
> > > dead. -- Aristotle
> > > _______________________________________________
> > > 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".
>
> > matroskadec.c | 62 +++++++++++++++++++++++++++++++++++++++++-----------------
> > 1 file changed, 44 insertions(+), 18 deletions(-)
> > a1ae98fa5d8dabe59ff6c3c8822aaef19ce980e6 0001-avformat-matroskadec-properly-parse-stsd-in-v_quickt.patch
> > From 9d639bea980bb7940bfeb527a60c847c14d13ee9 Mon Sep 17 00:00:00 2001
> > From: Stanislav Ionascu <stanislav.ionascu at gmail.com>
> > Date: Sun, 11 Aug 2019 21:10:30 +0200
> > Subject: [PATCH] avformat/matroskadec: properly parse stsd in v_quicktime
> >
> > 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.
> > QuickTime palette parsing is reused from the stsd parser results.
> >
> > Signed-off-by: Stanislav Ionascu <stanislav.ionascu at gmail.com>
> > ---
> > libavformat/matroskadec.c | 62 +++++++++++++++++++++++++++------------
> > 1 file changed, 44 insertions(+), 18 deletions(-)
>
> breaks:
> ./ffmpeg -i tickets//3256/output-ffmpeg-20120804-git-f857465.mkv -f null -
>
> https://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket3256/output-ffmpeg-20120804-git-f857465.mkv
Thanks. Brought back the workaround for the non-compliant private track data.
Patch attached.
Stan.
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Good people do not need laws to tell them to act responsibly, while bad
> people will find a way around the laws. -- Plato
> _______________________________________________
> 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: 4396 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190817/93752028/attachment.obj>
More information about the ffmpeg-devel
mailing list