[FFmpeg-devel] [PATCH] [HACK] mxfdec: support EIA-608 (closed caption subtitles).

Reimar Döffinger Reimar.Doeffinger at gmx.de
Fri Apr 20 20:03:02 CEST 2012


On Thu, Apr 19, 2012 at 02:05:31PM +0200, Tomas Härdin wrote:
> On Thu, 2012-04-12 at 21:45 +0200, Reimar Döffinger wrote:
> > This is a hack because:
> > 1) The way to detect this subtitle type is very questionable,
> >    and might cause other subtitle types to be misdetected
> >    as EIA-608
> 
> You should check EssenceContainer or whatever is the equivalent to
> PictureEssenceCoding for subtitles.

I didn't find anything relevant, though I might just miss the
right identifier to look for.

> > 2) The subtitle packets do not get any pts/dts values assigned
> 
> I suspect mxf->current_edit_unit should suffice.

I had a hack to just make it use the video code, but I didn't see
value in cluttering this initial patch with more guesses that
are not necessary for a working proof-of-concept.

> > 3) I don't have the slightest clue what those extra 32 bytes
> >    I throw away are good for
> 
> Please get a clue about that. In fact, try to find the relevant specs
> for this (both the subtitle spec and the corresponding SMPTE mapping
> document).

I couldn't find anything useful, that's the main reason for sending this
to the list.

> > --- a/libavformat/avienc.c
> > +++ b/libavformat/avienc.c
> > @@ -234,7 +234,7 @@ static int avi_write_header(AVFormatContext *s)
> >          case AVMEDIA_TYPE_SUBTITLE:
> >              // XSUB subtitles behave like video tracks, other subtitles
> >              // are not (yet) supported.
> > -            if (stream->codec_id != CODEC_ID_XSUB) {
> > +            if (0 && stream->codec_id != CODEC_ID_XSUB) {
> >                  av_log(s, AV_LOG_ERROR, "Subtitle streams other than DivX XSUB are not supported by the AVI muxer.\n");
> >                  return AVERROR_PATCHWELCOME;
> >              }
> 
> Unrelated?

Was just some debugging help.

> > @@ -2043,6 +2045,12 @@ static int mxf_read_packet_old(AVFormatContext *s, AVPacket *pkt)
> >                      av_log(s, AV_LOG_ERROR, "error reading D-10 aes3 frame\n");
> >                      return AVERROR_INVALIDDATA;
> >                  }
> > +            } else if (st->codec->codec_id == CODEC_ID_EIA_608 && klv.length > 28 + 4) {
> > +                avio_skip(s->pb, 28);
> > +                ret = av_get_packet(s->pb, pkt, klv.length - 28 - 4);
> > +                if (ret < 0)
> > +                    return ret;
> > +                avio_skip(s->pb, 4);
> >              } else {
> >                  ret = av_get_packet(s->pb, pkt, klv.length);
> >                  if (ret < 0)
> > @@ -2117,7 +2125,14 @@ static int mxf_read_packet(AVFormatContext *s, AVPacket *pkt)
> >      if ((ret64 = avio_seek(s->pb, pos, SEEK_SET)) < 0)
> >          return ret64;
> >  
> > -    if ((ret = av_get_packet(s->pb, pkt, size)) < 0)
> > +    if (st->codec->codec_id == CODEC_ID_EIA_608 && size > 28 + 4) {
> > +        avio_skip(s->pb, 28);
> > +        ret = av_get_packet(s->pb, pkt, size - 28 - 4);
> > +        avio_skip(s->pb, 4);
> > +    } else
> > +        ret = av_get_packet(s->pb, pkt, size);
> > +
> > +    if (ret < 0)
> 
> Fold these two hunks into a common function.

I'm not sure that will give a nice result.
More importantly though there's no point in it until it's clear what
should be in there (just skipping seems unlikely to really be the right
approach).


More information about the ffmpeg-devel mailing list