[FFmpeg-devel] [PATCH] Add AQTitle subtitles demuxer.
Clément Bœsch
ubitux at gmail.com
Sun Dec 30 23:41:44 CET 2012
On Sun, Dec 30, 2012 at 02:31:47PM +0100, Nicolas George wrote:
[...]
> > +static int aqt_probe(AVProbeData *p)
> > +{
> > + int frame;
> > + const char *ptr = p->buf;
> > +
>
> > + if (sscanf(ptr, "-->> %d", &frame) == 1)
> > + return AVPROBE_SCORE_MAX;
>
> Seems a little bit lax, does it not?
>
Yeah, now AVPROBE_SCORE_MAX / 2.
> > + return 0;
> > +}
> > +
> > +static int aqt_read_header(AVFormatContext *s)
> > +{
> > + AQTitleContext *aqt = s->priv_data;
> > + AVStream *st = avformat_new_stream(s, NULL);
> > + int new_event = 1;
> > + int64_t pos = 0, frame = AV_NOPTS_VALUE;
> > + AVPacket *sub = NULL;
> > +
> > + if (!st)
> > + return AVERROR(ENOMEM);
> > + avpriv_set_pts_info(st, 64, aqt->frame_rate.den, aqt->frame_rate.num);
> > + st->codec->codec_type = AVMEDIA_TYPE_SUBTITLE;
> > + st->codec->codec_id = AV_CODEC_ID_TEXT;
> > +
> > + while (!url_feof(s->pb)) {
> > + char line[4096];
> > + int len = ff_get_line(s->pb, line, sizeof(line));
> > +
> > + if (!len)
> > + break;
> > +
>
> > + line[strcspn(line, "\r\n")] = 0;
>
> Probably very inefficient, but it should not matter much.
>
Yeah… but I really like this trick!
> > +
> > + if (sscanf(line, "-->> %"PRId64, &frame) == 1) {
> > + new_event = 1;
> > + pos = avio_tell(s->pb);
>
> Is AVPacket.pos supposed to be the offset of the packet, or of the payload
> in the packet?
>
I doesn't really matter; it helps for the sort(), and seeking doesn't make
use of it. I tend to set the position at the start of the chunk (timers
pointers). Here it's just below, because it was simpler.
> > + } else if (*line) {
> > + if (!new_event) {
> > + sub = ff_subtitles_queue_insert(&aqt->q, "\n", 1, 1);
> > + if (!sub)
> > + return AVERROR(ENOMEM);
> > + }
> > + sub = ff_subtitles_queue_insert(&aqt->q, line, strlen(line), !new_event);
> > + if (!sub)
> > + return AVERROR(ENOMEM);
> > + if (new_event) {
> > + sub->pts = frame;
> > + sub->duration = -1;
> > + sub->pos = pos;
> > + }
> > + new_event = 0;
> > + } else if (sub) {
> > + sub->duration = frame - sub->pts;
> > + }
>
> I am not sure about the logic of this, especially on slightly broken files
> (empty lines in the middle of a subtitle or an empty line that is not really
> empty). Maybe move the "if (sub) sub->duration = ..." part in the "-->> %d"
> case (and set sub to NULL afterwards) ?
>
Ah you're right, fixed. Thanks.
> > + }
> > +
> > + ff_subtitles_queue_finalize(&aqt->q);
> > + return 0;
> > +}
> > +
> > +static int aqt_read_packet(AVFormatContext *s, AVPacket *pkt)
> > +{
> > + AQTitleContext *aqt = s->priv_data;
> > + return ff_subtitles_queue_read_packet(&aqt->q, pkt);
> > +}
> > +
> > +static int aqt_read_seek(AVFormatContext *s, int stream_index,
> > + int64_t min_ts, int64_t ts, int64_t max_ts, int flags)
> > +{
> > + AQTitleContext *aqt = s->priv_data;
> > + return ff_subtitles_queue_seek(&aqt->q, s, stream_index,
> > + min_ts, ts, max_ts, flags);
> > +}
> > +
> > +static int aqt_read_close(AVFormatContext *s)
> > +{
> > + AQTitleContext *aqt = s->priv_data;
> > + ff_subtitles_queue_clean(&aqt->q);
> > + return 0;
> > +}
> > +
> > +#define OFFSET(x) offsetof(AQTitleContext, x)
> > +#define SD AV_OPT_FLAG_SUBTITLE_PARAM|AV_OPT_FLAG_DECODING_PARAM
> > +static const AVOption aqt_options[] = {
> > + { "subtitles_fps", "set the movie frame rate", OFFSET(frame_rate), AV_OPT_TYPE_RATIONAL, {.dbl=25}, 0, INT_MAX, SD },
>
> Maybe a little too verbose? I believe "fps" should be enough; stream
> specifiers are explicit enough: "fpt:s".
>
Renamed to subfps, fps really is too wide IMO.
> > + { NULL }
> > +};
> > +
> > +static const AVClass aqt_class = {
> > + .class_name = "aqtdec",
> > + .item_name = av_default_item_name,
> > + .option = aqt_options,
> > + .version = LIBAVUTIL_VERSION_INT,
> > +};
> > +
> > +AVInputFormat ff_aqtitle_demuxer = {
> > + .name = "aqtitle",
> > + .long_name = NULL_IF_CONFIG_SMALL("AQTitle subtitles"),
> > + .priv_data_size = sizeof(AQTitleContext),
> > + .read_probe = aqt_probe,
> > + .read_header = aqt_read_header,
> > + .read_packet = aqt_read_packet,
> > + .read_seek2 = aqt_read_seek,
> > + .read_close = aqt_read_close,
> > + .extensions = "aqt",
> > + .priv_class = &aqt_class,
> > +};
> > diff --git a/libavformat/version.h b/libavformat/version.h
> > index 00a84be..d3c0d2a 100644
> > --- a/libavformat/version.h
> > +++ b/libavformat/version.h
> > @@ -30,7 +30,7 @@
> > #include "libavutil/avutil.h"
> >
> > #define LIBAVFORMAT_VERSION_MAJOR 54
> > -#define LIBAVFORMAT_VERSION_MINOR 53
> > +#define LIBAVFORMAT_VERSION_MINOR 54
> > #define LIBAVFORMAT_VERSION_MICRO 100
> >
> > #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> > diff --git a/tests/fate/subtitles.mak b/tests/fate/subtitles.mak
> > index b664386..0887d1c 100644
> > --- a/tests/fate/subtitles.mak
> > +++ b/tests/fate/subtitles.mak
> > @@ -1,3 +1,6 @@
> > +FATE_SUBTITLES_ASS-$(call DEMDEC, AQTITLE, TEXT) += fate-sub-aqtitle
> > +fate-sub-aqtitle: CMD = md5 -i $(SAMPLES)/sub/AQTitle_capability_tester.aqt -f ass
> > +
> > FATE_SUBTITLES_ASS-$(call DEMDEC, JACOSUB, JACOSUB) += fate-sub-jacosub
> > fate-sub-jacosub: CMD = md5 -i $(SAMPLES)/sub/JACOsub_capability_tester.jss -f ass
> >
> > diff --git a/tests/ref/fate/sub-aqtitle b/tests/ref/fate/sub-aqtitle
> > new file mode 100644
> > index 0000000..5c57f80
> > --- /dev/null
> > +++ b/tests/ref/fate/sub-aqtitle
> > @@ -0,0 +1 @@
> > +0dbaa163f58fd156c48b19c7e45046c3
>
> Regards,
>
Applied, thanks.
--
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121230/e97fcde3/attachment.asc>
More information about the ffmpeg-devel
mailing list