Re: [FFmpeg-devel] [PATCH 1/1] avformat/matroska: fully parse stsd atom in v_quicktime tracks
Hi!
On Tue, Aug 20, 2019 at 10:19 AM Michael Niedermayer
wrote:
>
> On Sun, Aug 18, 2019 at 12:32:03PM +0200, Stanislav Ionascu wrote:
> > Hi,
> >
> > thanks for looking into this.
> >
> > On Sun, Aug 18, 2019 at 4:55 AM Andreas Rheinhardt
> > wrote:
> > >
> > > Hello,
> > >
> > > I am no expert on mov (and so this should definitely be looked at from
> > > someone who is), but I have some points:
> > >
> > > Stanislav Ionascu:
> > > > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> > > > index 1ea9b807e6..2a397c909a 100644
> > > > --- a/libavformat/matroskadec.c
> > > > +++ b/libavformat/matroskadec.c
> > > > @@ -2473,25 +2473,58 @@ 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;
> > > > +void *priv_data;
> > > > +int nb_streams;
> > >
> > > You could initialize nb_streams and priv_data here. And the
> > > initialization of mc is unnecessary.
> >
> > Done.
> >
> > >
> > > > int ret = get_qt_codec(track, , _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);
> > > > +mc = av_mallocz(sizeof(*mc));
> > > > +if (!mc)
> > > > +return AVERROR(ENOMEM);
> > > > +priv_data = st->priv_data;
> > > > +nb_streams = s->nb_streams;
> > > > +mc->fc = s;
> > > > +st->priv_data = msc = av_mallocz(sizeof(MOVStreamContext));
> > > > +if (!msc) {
> > > > +av_free(mc);
> > > > +st->priv_data = priv_data;
> > > > +return AVERROR(ENOMEM);
> > > > }
> > > > +ffio_init_context(, track->codec_priv.data,
> > > > + track->codec_priv.size,
> > > > + 0, NULL, NULL, NULL, NULL);
> > > > +
> > > > +/* 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, , 1);
> > >
> > > Is it intentional that you don't check the return value here?.
> >
> > Added the check.
> >
> > >
> > > > +
> > > > +/* copy palette from MOVStreamContext */
> > > > +track->has_palette = msc->has_palette;
> > > > +if (track->has_palette) {
> > > > +/* leave bit_depth = -1, to reuse
> > > bits_per_coded_sample */
> > > > +memcpy(track->palette, msc->palette, AVPALETTE_COUNT);
> > >
> > > In case the track has a palette, your patch would only copy 256 bytes
> > > of it, but a palette consists of 256 uint32_t. (This link
> > > https://drive.google.com/drive/folders/0B3_pEBoLs0faWElmM2FnLTZYNlk
> > > from the commit message that added the palette stuff seems to still
> > > work if you need samples for this.)
> >
> > Indeed. I've corrected that. Also remuxed all mov's into mkv's, and
> > verified that they all still work.
> >
> > >
> > > > +}
> > > > +
> > > > +av_free(msc);
> > > > +av_free(mc);
> > > > +st->priv_data = priv_data;
> > > > +s->nb_streams = nb_streams;
> > > > +fourcc = st->codecpar->codec_tag;
> > > > +codec_id = st->codecpar->codec_id;
> > > > +
> > > > +av_log(matroska->ctx, AV_LOG_TRACE,
> > > > + "mov FourCC found %s.\n", av_fourcc2str(fourcc));
> > > > +
> > > > if (codec_id == AV_CODEC_ID_NONE)
> > > > av_log(matroska->ctx, AV_LOG_ERROR,
> > > > - "mov FourCC not found %s.\n",
> > > av_fourcc2str(fourcc));
> > >
> > > If the codec_id turns out to be AV_CODEC_ID_NONE, two strings will be
> > > output (at least on loglevel trace): "mov FourCC found %s.\n" and then
> > > "mov FourCC not found %s.\n". The first of these is superfluous in
> > > this case.
> >
> > Removed it, as stsd parser will also trace the FourCC code.
> >
> > >
> > > > -if (track->codec_priv.size >= 86) {
> > > > -bit_depth = AV_RB16(track->codec_priv.data + 82);
> > > > -ffio_init_context(, track->codec_priv.data,
> > > > - track->codec_priv.size,
> > > > - 0, NULL, NULL, NULL, NULL);
> > > > -if (ff_get_qtpalette(codec_id, , track->palette)) {
> > >
> > > If I
Re: [FFmpeg-devel] [PATCH 1/1] avformat/matroska: fully parse stsd atom in v_quicktime tracks
On Sun, Aug 18, 2019 at 12:32:03PM +0200, Stanislav Ionascu wrote:
> Hi,
>
> thanks for looking into this.
>
> On Sun, Aug 18, 2019 at 4:55 AM Andreas Rheinhardt
> wrote:
> >
> > Hello,
> >
> > I am no expert on mov (and so this should definitely be looked at from
> > someone who is), but I have some points:
> >
> > Stanislav Ionascu:
> > > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> > > index 1ea9b807e6..2a397c909a 100644
> > > --- a/libavformat/matroskadec.c
> > > +++ b/libavformat/matroskadec.c
> > > @@ -2473,25 +2473,58 @@ 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;
> > > +void *priv_data;
> > > +int nb_streams;
> >
> > You could initialize nb_streams and priv_data here. And the
> > initialization of mc is unnecessary.
>
> Done.
>
> >
> > > int ret = get_qt_codec(track, , _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);
> > > +mc = av_mallocz(sizeof(*mc));
> > > +if (!mc)
> > > +return AVERROR(ENOMEM);
> > > +priv_data = st->priv_data;
> > > +nb_streams = s->nb_streams;
> > > +mc->fc = s;
> > > +st->priv_data = msc = av_mallocz(sizeof(MOVStreamContext));
> > > +if (!msc) {
> > > +av_free(mc);
> > > +st->priv_data = priv_data;
> > > +return AVERROR(ENOMEM);
> > > }
> > > +ffio_init_context(, track->codec_priv.data,
> > > + track->codec_priv.size,
> > > + 0, NULL, NULL, NULL, NULL);
> > > +
> > > +/* 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, , 1);
> >
> > Is it intentional that you don't check the return value here?.
>
> Added the check.
>
> >
> > > +
> > > +/* copy palette from MOVStreamContext */
> > > +track->has_palette = msc->has_palette;
> > > +if (track->has_palette) {
> > > +/* leave bit_depth = -1, to reuse
> > bits_per_coded_sample */
> > > +memcpy(track->palette, msc->palette, AVPALETTE_COUNT);
> >
> > In case the track has a palette, your patch would only copy 256 bytes
> > of it, but a palette consists of 256 uint32_t. (This link
> > https://drive.google.com/drive/folders/0B3_pEBoLs0faWElmM2FnLTZYNlk
> > from the commit message that added the palette stuff seems to still
> > work if you need samples for this.)
>
> Indeed. I've corrected that. Also remuxed all mov's into mkv's, and
> verified that they all still work.
>
> >
> > > +}
> > > +
> > > +av_free(msc);
> > > +av_free(mc);
> > > +st->priv_data = priv_data;
> > > +s->nb_streams = nb_streams;
> > > +fourcc = st->codecpar->codec_tag;
> > > +codec_id = st->codecpar->codec_id;
> > > +
> > > +av_log(matroska->ctx, AV_LOG_TRACE,
> > > + "mov FourCC found %s.\n", av_fourcc2str(fourcc));
> > > +
> > > if (codec_id == AV_CODEC_ID_NONE)
> > > av_log(matroska->ctx, AV_LOG_ERROR,
> > > - "mov FourCC not found %s.\n",
> > av_fourcc2str(fourcc));
> >
> > If the codec_id turns out to be AV_CODEC_ID_NONE, two strings will be
> > output (at least on loglevel trace): "mov FourCC found %s.\n" and then
> > "mov FourCC not found %s.\n". The first of these is superfluous in
> > this case.
>
> Removed it, as stsd parser will also trace the FourCC code.
>
> >
> > > -if (track->codec_priv.size >= 86) {
> > > -bit_depth = AV_RB16(track->codec_priv.data + 82);
> > > -ffio_init_context(, track->codec_priv.data,
> > > - track->codec_priv.size,
> > > - 0, NULL, NULL, NULL, NULL);
> > > -if (ff_get_qtpalette(codec_id, , track->palette)) {
> >
> > If I am not extremely mistaken, then there is no need to include
> > qtpalette.h any more after removing this function call.
>
> Yes. Removed as it's not necessary.
>
> >
> > > -bit_depth &= 0x1F;
> > > -track->has_palette = 1;
> > > -}
> > > +
Re: [FFmpeg-devel] [PATCH 1/1] avformat/matroska: fully parse stsd atom in v_quicktime tracks
Hi,
thanks for looking into this.
On Sun, Aug 18, 2019 at 4:55 AM Andreas Rheinhardt
wrote:
>
> Hello,
>
> I am no expert on mov (and so this should definitely be looked at from
> someone who is), but I have some points:
>
> Stanislav Ionascu:
> > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> > index 1ea9b807e6..2a397c909a 100644
> > --- a/libavformat/matroskadec.c
> > +++ b/libavformat/matroskadec.c
> > @@ -2473,25 +2473,58 @@ 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;
> > +void *priv_data;
> > +int nb_streams;
>
> You could initialize nb_streams and priv_data here. And the
> initialization of mc is unnecessary.
Done.
>
> > int ret = get_qt_codec(track, , _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);
> > +mc = av_mallocz(sizeof(*mc));
> > +if (!mc)
> > +return AVERROR(ENOMEM);
> > +priv_data = st->priv_data;
> > +nb_streams = s->nb_streams;
> > +mc->fc = s;
> > +st->priv_data = msc = av_mallocz(sizeof(MOVStreamContext));
> > +if (!msc) {
> > +av_free(mc);
> > +st->priv_data = priv_data;
> > +return AVERROR(ENOMEM);
> > }
> > +ffio_init_context(, track->codec_priv.data,
> > + track->codec_priv.size,
> > + 0, NULL, NULL, NULL, NULL);
> > +
> > +/* 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, , 1);
>
> Is it intentional that you don't check the return value here?.
Added the check.
>
> > +
> > +/* copy palette from MOVStreamContext */
> > +track->has_palette = msc->has_palette;
> > +if (track->has_palette) {
> > +/* leave bit_depth = -1, to reuse
> bits_per_coded_sample */
> > +memcpy(track->palette, msc->palette, AVPALETTE_COUNT);
>
> In case the track has a palette, your patch would only copy 256 bytes
> of it, but a palette consists of 256 uint32_t. (This link
> https://drive.google.com/drive/folders/0B3_pEBoLs0faWElmM2FnLTZYNlk
> from the commit message that added the palette stuff seems to still
> work if you need samples for this.)
Indeed. I've corrected that. Also remuxed all mov's into mkv's, and
verified that they all still work.
>
> > +}
> > +
> > +av_free(msc);
> > +av_free(mc);
> > +st->priv_data = priv_data;
> > +s->nb_streams = nb_streams;
> > +fourcc = st->codecpar->codec_tag;
> > +codec_id = st->codecpar->codec_id;
> > +
> > +av_log(matroska->ctx, AV_LOG_TRACE,
> > + "mov FourCC found %s.\n", av_fourcc2str(fourcc));
> > +
> > if (codec_id == AV_CODEC_ID_NONE)
> > av_log(matroska->ctx, AV_LOG_ERROR,
> > - "mov FourCC not found %s.\n",
> av_fourcc2str(fourcc));
>
> If the codec_id turns out to be AV_CODEC_ID_NONE, two strings will be
> output (at least on loglevel trace): "mov FourCC found %s.\n" and then
> "mov FourCC not found %s.\n". The first of these is superfluous in
> this case.
Removed it, as stsd parser will also trace the FourCC code.
>
> > -if (track->codec_priv.size >= 86) {
> > -bit_depth = AV_RB16(track->codec_priv.data + 82);
> > -ffio_init_context(, track->codec_priv.data,
> > - track->codec_priv.size,
> > - 0, NULL, NULL, NULL, NULL);
> > -if (ff_get_qtpalette(codec_id, , track->palette)) {
>
> If I am not extremely mistaken, then there is no need to include
> qtpalette.h any more after removing this function call.
Yes. Removed as it's not necessary.
>
> > -bit_depth &= 0x1F;
> > -track->has_palette = 1;
> > -}
> > +"mov FourCC not found %s.\n",
> av_fourcc2str(fourcc));
> > +
> > +// dvh1 in mkv is likely HEVC
> > +if (fourcc == MKTAG('d','v','h','1')) {
> > +codec_id = AV_CODEC_ID_HEVC;
>
> Your changes for dvh1 should probably be moved to a separate commit.
Removed.
Re: [FFmpeg-devel] [PATCH 1/1] avformat/matroska: fully parse stsd atom in v_quicktime tracks
Hello,
I am no expert on mov (and so this should definitely be looked at from
someone who is), but I have some points:
Stanislav Ionascu:
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 1ea9b807e6..2a397c909a 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -2473,25 +2473,58 @@ 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;
> +void *priv_data;
> +int nb_streams;
You could initialize nb_streams and priv_data here. And the
initialization of mc is unnecessary.
> int ret = get_qt_codec(track, , _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);
> +mc = av_mallocz(sizeof(*mc));
> +if (!mc)
> +return AVERROR(ENOMEM);
> +priv_data = st->priv_data;
> +nb_streams = s->nb_streams;
> +mc->fc = s;
> +st->priv_data = msc = av_mallocz(sizeof(MOVStreamContext));
> +if (!msc) {
> +av_free(mc);
> +st->priv_data = priv_data;
> +return AVERROR(ENOMEM);
> }
> +ffio_init_context(, track->codec_priv.data,
> + track->codec_priv.size,
> + 0, NULL, NULL, NULL, NULL);
> +
> +/* 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, , 1);
Is it intentional that you don't check the return value here?
> +
> +/* copy palette from MOVStreamContext */
> +track->has_palette = msc->has_palette;
> +if (track->has_palette) {
> +/* leave bit_depth = -1, to reuse
bits_per_coded_sample */
> +memcpy(track->palette, msc->palette, AVPALETTE_COUNT);
In case the track has a palette, your patch would only copy 256 bytes
of it, but a palette consists of 256 uint32_t. (This link
https://drive.google.com/drive/folders/0B3_pEBoLs0faWElmM2FnLTZYNlk
from the commit message that added the palette stuff seems to still
work if you need samples for this.)
> +}
> +
> +av_free(msc);
> +av_free(mc);
> +st->priv_data = priv_data;
> +s->nb_streams = nb_streams;
> +fourcc = st->codecpar->codec_tag;
> +codec_id = st->codecpar->codec_id;
> +
> +av_log(matroska->ctx, AV_LOG_TRACE,
> + "mov FourCC found %s.\n", av_fourcc2str(fourcc));
> +
> if (codec_id == AV_CODEC_ID_NONE)
> av_log(matroska->ctx, AV_LOG_ERROR,
> - "mov FourCC not found %s.\n",
av_fourcc2str(fourcc));
If the codec_id turns out to be AV_CODEC_ID_NONE, two strings will be
output (at least on loglevel trace): "mov FourCC found %s.\n" and then
"mov FourCC not found %s.\n". The first of these is superfluous in
this case.
> -if (track->codec_priv.size >= 86) {
> -bit_depth = AV_RB16(track->codec_priv.data + 82);
> -ffio_init_context(, track->codec_priv.data,
> - track->codec_priv.size,
> - 0, NULL, NULL, NULL, NULL);
> -if (ff_get_qtpalette(codec_id, , track->palette)) {
If I am not extremely mistaken, then there is no need to include
qtpalette.h any more after removing this function call.
> -bit_depth &= 0x1F;
> -track->has_palette = 1;
> -}
> +"mov FourCC not found %s.\n",
av_fourcc2str(fourcc));
> +
> +// dvh1 in mkv is likely HEVC
> +if (fourcc == MKTAG('d','v','h','1')) {
> +codec_id = AV_CODEC_ID_HEVC;
Your changes for dvh1 should probably be moved to a separate commit.
> }
> } else if (codec_id == AV_CODEC_ID_PCM_S16BE) {
> switch (track->audio.bitdepth) {
- Andreas
___
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".
Re: [FFmpeg-devel] [PATCH 1/1] avformat/matroska: fully parse stsd atom in v_quicktime tracks
Hi!
On Sat, Aug 17, 2019 at 12:53 AM Michael Niedermayer
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
> > 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
> > > > 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
> > > > > > ---
> > > > > > 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, , _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(, track->codec_priv.data,
> > > > > > - track->codec_priv.size,
> > > > > > - 0, NULL, NULL, NULL, NULL);
> > > > > > -if (ff_get_qtpalette(codec_id, ,
> > > > > > 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. */
> > > > > > +
Re: [FFmpeg-devel] [PATCH 1/1] avformat/matroska: fully parse stsd atom in v_quicktime tracks
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
> 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
> > > 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
> > > > > ---
> > > > > 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, , _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(, track->codec_priv.data,
> > > > > - track->codec_priv.size,
> > > > > - 0, NULL, NULL, NULL, NULL);
> > > > > -if (ff_get_qtpalette(codec_id, , 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(_ctx);
> > > > > +st->priv_data = priv_data;
> > > > > +s->nb_streams =
Re: [FFmpeg-devel] [PATCH 1/1] avformat/matroska: fully parse stsd atom in v_quicktime tracks
Hi,
On Wed, Aug 14, 2019 at 11:50 PM Michael Niedermayer
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
> > 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
> > > > ---
> > > > 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, , _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(, track->codec_priv.data,
> > > > - track->codec_priv.size,
> > > > - 0, NULL, NULL, NULL, NULL);
> > > > -if (ff_get_qtpalette(codec_id, , 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(_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) {
> > > >
Re: [FFmpeg-devel] [PATCH 1/1] avformat/matroska: fully parse stsd atom in v_quicktime tracks
On Wed, Aug 14, 2019 at 08:44:11PM +0200, Stanislav Ionascu wrote:
> On Tue, Aug 13, 2019 at 10:22 PM Andreas Rheinhardt
> 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
> > > ---
> > > 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, , _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(, track->codec_priv.data,
> > > - track->codec_priv.size,
> > > - 0, NULL, NULL, NULL, NULL);
> > > -if (ff_get_qtpalette(codec_id, , 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(_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
>
Re: [FFmpeg-devel] [PATCH 1/1] avformat/matroska: fully parse stsd atom in v_quicktime tracks
On Tue, Aug 13, 2019 at 10:22 PM Andreas Rheinhardt
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
> > ---
> > 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, , _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(, track->codec_priv.data,
> > - track->codec_priv.size,
> > - 0, NULL, NULL, NULL, NULL);
> > -if (ff_get_qtpalette(codec_id, , 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(_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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
0001-avformat-matroskadec-properly-parse-stsd-in-v_quickt.patch
Description: Binary data
Re: [FFmpeg-devel] [PATCH 1/1] avformat/matroska: fully parse stsd atom in v_quicktime tracks
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
> ---
> 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, , _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.
> +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(, track->codec_priv.data,
> - track->codec_priv.size,
> - 0, NULL, NULL, NULL, NULL);
> -if (ff_get_qtpalette(codec_id, , track->palette)) {
> -bit_depth &= 0x1F;
> -track->has_palette = 1;
Why are you removing this code? What about tracks that ought to have a
palette?
> -}
> +/* 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(_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.
- Andreas
___
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".
[FFmpeg-devel] [PATCH 1/1] avformat/matroska: fully parse stsd atom in v_quicktime tracks
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
---
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, , _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);
+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(, track->codec_priv.data,
- track->codec_priv.size,
- 0, NULL, NULL, NULL, NULL);
-if (ff_get_qtpalette(codec_id, , track->palette)) {
-bit_depth &= 0x1F;
-track->has_palette = 1;
-}
+/* 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(_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) {
--
2.20.1
___
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".
