[FFmpeg-devel] [PATCH] Populate MPEG2TS codec_tag using ff_codec_movvideo_tags/ff_codec_movaudio_tags (av4cc) rather than the PES stream_type. For MPEG2TS files containing h264, ffprobe currently returns codec_tag_string=[27][0][0][0], this change will now produce codec_tag_string=avc1

[Damien Lévin]

Thanks Hendrik,

The documentation from the AVCodecParameters codec_tag changed here (and
not AVCodecContext which you are referring to) seems pretty explicit about
being an AVI fourcc. So I'm not sure I understand why setting the PES
stream_type (ISO/IEC 13818-1, table 2-34) would be more correct.
"Additional information about the codec (corresponds to the AVI FOURCC)."

If you look at the code from ffprobe.c (which also uses  AVCodecParameters)
the assumption is to also have an AVI fourcc.

/* print AVI/FourCC tag */
print_str("codec_tag_string", av_fourcc2str(par->codec_tag));
print_fmt("codec_tag", "0x%04"PRIx32, par->codec_tag);

I understand that dumping the stream_type can be convenient but that
doesn't seems like the correct behavior.
Let me know what you think.

Damien

On Wed, May 15, 2019, 2:25 PM Hendrik Leppkes <h.leppkes at gmail.com> wrote:

> On Wed, May 15, 2019 at 6:15 PM Damien Levin
> <dlevin-at-google.com at ffmpeg.org> wrote:
> >
> > ---
> >  libavformat/mpegts.c                          | 9 +++++++--
> >  tests/ref/fate/concat-demuxer-simple2-lavf-ts | 4 ++--
> >  2 files changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> > index 8a84e5cc19..79c0b78b1f 100644
> > --- a/libavformat/mpegts.c
> > +++ b/libavformat/mpegts.c
> > @@ -877,6 +877,13 @@ static void mpegts_find_stream_type(AVStream *st,
> >                  st->codecpar->codec_id   != types->codec_id) {
> >                  st->codecpar->codec_type = types->codec_type;
> >                  st->codecpar->codec_id   = types->codec_id;
> > +                if (types->codec_type == AVMEDIA_TYPE_VIDEO) {
> > +                    st->codecpar->codec_tag =
> > +                        ff_codec_get_tag(ff_codec_movvideo_tags,
> types->codec_id);
> > +                } else if (types->codec_type == AVMEDIA_TYPE_AUDIO) {
> > +                    st->codecpar->codec_tag =
> > +                        ff_codec_get_tag(ff_codec_movaudio_tags,
> types->codec_id);
> > +                }
> >                  st->internal->need_context_update = 1;
> >              }
> >              st->request_probe        = 0;
> > @@ -908,8 +915,6 @@ static int mpegts_set_stream_info(AVStream *st,
> PESContext *pes,
> >             "stream=%d stream_type=%x pid=%x prog_reg_desc=%.4s\n",
> >             st->index, pes->stream_type, pes->pid, (char
> *)&prog_reg_desc);
> >
> > -    st->codecpar->codec_tag = pes->stream_type;
> > -
>
> The current behavior is correct, per the documentation of the field:
> " A demuxer should set this to what is stored in the field used to
> identify the codec."
>
> Arbitrarily setting the field from a table without any relation to the
> container is definitely not correct. If a user wants to know the tag
> to be used in mov/mp4, they can determine that independently, but not
> from the mpegts demuxer. This field is container-specific, and as such
> exporting the pes stream type is actually the correct thing to do.
>
> On a personal note, I actually use that value to detect certain
> streams from Blu-rays and their secondary meaning, since every pes
> stream type is clearly defined, the value can be used to eg. identify
> if an audio stream is primary or secondary audio.
>
> - Hendrik
> _______________________________________________
> 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".