diff mbox

[FFmpeg-devel,6/6] avformat/mxfdec: add support for recognizing timed text streams

Message ID 20180531000536.11482-6-cus@passwd.hu
State New
Headers show

Commit Message

Marton Balint May 31, 2018, 12:05 a.m. UTC 
Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mxfdec.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Tomas Härdin June 5, 2018, 11:31 a.m. UTC | #1 
tor 2018-05-31 klockan 02:05 +0200 skrev Marton Balint:
> > Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/mxfdec.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> +            type = avcodec_get_type(st->codecpar->codec_id);
> +            if (type == AVMEDIA_TYPE_SUBTITLE)
> +                st->codecpar->codec_type = type;

This feels like something that belongs in more generic parts of lavf.
Filling codecpar with defaults for some given codec_id that is.

/Tomas
Marton Balint June 5, 2018, 6:33 p.m. UTC | #2 
On Tue, 5 Jun 2018, Tomas Härdin wrote:

> tor 2018-05-31 klockan 02:05 +0200 skrev Marton Balint:
>> > Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavformat/mxfdec.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>> 
>> +            type = avcodec_get_type(st->codecpar->codec_id);
>> +            if (type == AVMEDIA_TYPE_SUBTITLE)
>> +                st->codecpar->codec_type = type;
>
> This feels like something that belongs in more generic parts of lavf.
> Filling codecpar with defaults for some given codec_id that is.

I assumed some codecs can be audio and video at the same time, and this is 
why it's not automatic. wrapped_avframe (like it or not) is such a codec. 
In any case, I'd rather not change the way it works here...

Regards,
Marton
Tomas Härdin June 6, 2018, 5:30 p.m. UTC | #3 
tis 2018-06-05 klockan 20:33 +0200 skrev Marton Balint:
> 
> On Tue, 5 Jun 2018, Tomas Härdin wrote:
> 
> > tor 2018-05-31 klockan 02:05 +0200 skrev Marton Balint:
> > > > Signed-off-by: Marton Balint <cus@passwd.hu>
> > > 
> > > ---
> > >  libavformat/mxfdec.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > > 
> > > +            type = avcodec_get_type(st->codecpar->codec_id);
> > > +            if (type == AVMEDIA_TYPE_SUBTITLE)
> > > +                st->codecpar->codec_type = type;
> > 
> > This feels like something that belongs in more generic parts of lavf.
> > Filling codecpar with defaults for some given codec_id that is.
> 
> I assumed some codecs can be audio and video at the same time, and this is 
> why it's not automatic. wrapped_avframe (like it or not) is such a codec. 
> In any case, I'd rather not change the way it works here...

Huh, didn't know that. Actually, the separation of audio and video is
probably one of the greatest crimes in digital multimedia

/Tomas
diff mbox

Patch

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index cf1cd71987..d9ce09cc75 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -1280,6 +1280,7 @@  static const MXFCodecUL mxf_sound_essence_container_uls[] = {
 static const MXFCodecUL mxf_data_essence_container_uls[] = {
     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x09,0x0d,0x01,0x03,0x01,0x02,0x0d,0x00,0x00 }, 16, AV_CODEC_ID_NONE,      "vbi_smpte_436M" },
     { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x09,0x0d,0x01,0x03,0x01,0x02,0x0e,0x00,0x00 }, 16, AV_CODEC_ID_NONE, "vbi_vanc_smpte_436M" },
+    { { 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x09,0x0d,0x01,0x03,0x01,0x02,0x13,0x01,0x01 }, 16, AV_CODEC_ID_TIMED_TEXT_MARKUP },
     { { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 },  0, AV_CODEC_ID_NONE },
 };
 
@@ -2351,7 +2352,13 @@  static int mxf_parse_structural_metadata(MXFContext *mxf)
                 st->need_parsing = AVSTREAM_PARSE_FULL;
             }
         } else if (st->codecpar->codec_type == AVMEDIA_TYPE_DATA) {
+            enum AVMediaType type;
             container_ul = mxf_get_codec_ul(mxf_data_essence_container_uls, essence_container_ul);
+            if (st->codecpar->codec_id == AV_CODEC_ID_NONE)
+                st->codecpar->codec_id = container_ul->id;
+            type = avcodec_get_type(st->codecpar->codec_id);
+            if (type == AVMEDIA_TYPE_SUBTITLE)
+                st->codecpar->codec_type = type;
             if (container_ul->desc)
                 av_dict_set(&st->metadata, "data_type", container_ul->desc, 0);
         }
@@ -2501,6 +2508,7 @@  static const MXFMetadataReadTableEntry mxf_metadata_read_table[] = {
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x5b,0x00 }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /* VBI - SMPTE 436M */
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x5c,0x00 }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /* VANC/VBI - SMPTE 436M */
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x5e,0x00 }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /* MPEG2AudioDescriptor */
+    { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x64,0x00 }, mxf_read_generic_descriptor, sizeof(MXFDescriptor), Descriptor }, /* DC Timed Text Descriptor */
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x3A,0x00 }, mxf_read_track, sizeof(MXFTrack), Track }, /* Static Track */
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x3B,0x00 }, mxf_read_track, sizeof(MXFTrack), Track }, /* Generic Track */
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x14,0x00 }, mxf_read_timecode_component, sizeof(MXFTimecodeComponent), TimecodeComponent },