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

[Tomas Härdin]

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.

> 2) The subtitle packets do not get any pts/dts values assigned

I suspect mxf->current_edit_unit should suffice.

> 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).

> --- 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?

> @@ -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 can't offer much more feedback at the moment due to sickness.

/Tomas