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, &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); > + 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(&b, 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, &b, 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(&b, track->codec_priv.data, > - track->codec_priv.size, > - 0, NULL, NULL, NULL, NULL); > - if (ff_get_qtpalette(codec_id, &b, 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".