Le vendredi 01 juin 2012 à 10:23 -0700, Alex Converse a écrit :
> On Fri, Jun 1, 2012 at 8:11 AM, David Girault <[email protected]>
> wrote:
> >
> > Added support for HDMV Interactive Graphics Stream in mpegts
> >
> > This new muxer/demuxer will allow to read/write menu stream
> > from/to 'mnu' files. This will allow to extract menu from BDAV
> > m2ts file to a elementary stream file suitable for Bluray
> > authoring software (such as BDedit from domm9).
> >
> > Mpeg TS muxer/demuxer was also updated to recognize these IGS
> > menu streams. More update required to allow muxing IGS menu
> > inside an mpegts file.
> >
> > Signed-off-by: David Girault <[email protected]>
> > ---
> > libavformat/Makefile | 2 +
> > libavformat/allformats.c | 1 +
> > libavformat/avformat.h | 1 +
> > libavformat/mnudec.c | 103
> > ++++++++++++++++++++++++++++++++++++++++++++++
> > libavformat/mnuenc.c | 64 ++++++++++++++++++++++++++++
> > libavformat/mpegts.c | 1 +
> > 6 files changed, 172 insertions(+)
> > create mode 100644 libavformat/mnudec.c
> > create mode 100644 libavformat/mnuenc.c
> >
>
> Seems pretty good overall but I have a few concerns about the probe.
>
> > diff --git a/libavformat/Makefile b/libavformat/Makefile
> [...]
> > diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> [...]
> > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> [...]
> > diff --git a/libavformat/mnudec.c b/libavformat/mnudec.c
> > new file mode 100644
> > index 0000000..857b0f1
> > --- /dev/null
> > +++ b/libavformat/mnudec.c
> > @@ -0,0 +1,103 @@
> > +/*
> > + * MNU demuxer
> > + * Copyright (c) 2012 David Girault
> > + *
> > + * This file is part of Libav.
> > + *
> > + * Libav is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * Libav is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with Libav; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > 02110-1301 USA
> > + */
> > +
> > +#include "libavutil/mathematics.h"
> > +#include "libavcodec/bytestream.h"
> > +#include "avformat.h"
> > +#include "internal.h"
> > +
> > +typedef struct MNUContext{
> > + uint8_t *buffer;
> > +} MNUContext;
> > +
> > +static int probe(AVProbeData *p)
> > +{
> > + const char *header= "IG";
>
> static const char header[]
ok. no problem. it's a better syntax for that kind anyway. ;)
>
> > +
> > + if(!memcmp(p->buf , header, strlen(header)))
>
> do we guarantee p->buf_size is always >= 2?
No. if we have a corrupted source. By the way, Next point will resolve
this issue.
>
> > + return AVPROBE_SCORE_MAX;
>
> Two bytes seems like very little for AVPROBE_SCORE_MAX? Can we score
> lower for just one packet and increase if we see 2+?
This format, to be valid, require at least 4 frames (or segments):
pages/buttons description segment, one palette segment, one picture
segment and the last, the display segment.
Usually, first segment may be large, more than probe buffer (which is 2k
if I remind well).
So, we may check length is at least enough for ('IG'+ 8 bytes PTS&DTS +
segment type + segment size) = 13 bytes. If IG and segment type is
valid, I think it should be OK for scoring max*3/4.
If probe buffer size is larger than first segment size, we may also
check the marker of the second segment and result with score max.
Is it ok for you?
Ps: There is some little questions at the end of the email not
concerning your comments....
>
> > +
> > + return 0;
> > +}
> > +
> > +static int read_close(AVFormatContext *s)
> > +{
> > + MNUContext *mnu = s->priv_data;
> > + av_freep(&mnu->buffer);
> > + return 0;
> > +}
> > +
> > +static int read_header(AVFormatContext *s)
> > +{
> > + AVStream *st = avformat_new_stream(s, NULL);
> > + if (!st)
> > + return -1;
>
> return AVERROR(ENOMEM)
>
> > + //avpriv_set_pts_info(st, 64, 1, 100);
> > + avpriv_set_pts_info(st, 33, 1, 90000);
> > + st->codec->codec_type = AVMEDIA_TYPE_OVERLAY;
> > + st->codec->codec_id= CODEC_ID_HDMV_IGS_MENU;
> > + return 0;
> > +}
> > +
> > +static int read_packet(AVFormatContext *s, AVPacket *pkt)
> > +{
> > +// MNUContext *mnu = s->priv_data;
> > + uint64_t pos = avio_tell(s->pb);
> > + int res = AVERROR_EOF;
> > +
> > + char header[2];
> > + uint32_t pts, dts;
> > + uint8_t seg_type;
> > + uint16_t seg_length;
> > +
> > + // "IG",PTS,DTS,SEG_TYPE,SEG_LENGTH
> > + res = avio_read(s->pb, header, 2);
> > + if (header[0]!='I'||header[1]!='G') return AVERROR_INVALIDDATA;
> > + pts = avio_rb32(s->pb);
> > + dts = avio_rb32(s->pb);
> > + seg_type = avio_r8(s->pb);
> > + seg_length = avio_rb16(s->pb);
> > +
> > + res = av_new_packet(pkt, seg_length+3);
> > + if (!res) {
> > + uint8_t *buf = pkt->data;
> > + bytestream_put_byte(&buf, seg_type);
> > + bytestream_put_be16(&buf, seg_length);
> > + avio_read(s->pb, buf, seg_length);
> > + pkt->flags |= AV_PKT_FLAG_KEY;
> > + pkt->pos = pos;
> > + pkt->pts = pts;
> > + pkt->dts = dts;
> > + av_dlog(s, "New IG packet @ 0x%lx: type %d, length %d\n",
> > + pos, seg_type, seg_length);
> > + }
> > + return res;
> > +}
> > +
> > +AVInputFormat ff_mnu_demuxer = {
> > + .name = "mnu",
> > + .long_name = NULL_IF_CONFIG_SMALL("HDMV Interactive Graphic
> > menus format"),
> > + .priv_data_size = sizeof(MNUContext),
> > + .read_probe = probe,
> > + .read_header = read_header,
> > + .read_packet = read_packet,
> > + .read_close = read_close,
> > +};
> > diff --git a/libavformat/mnuenc.c b/libavformat/mnuenc.c
> > new file mode 100644
> > index 0000000..15d0ff8
> > --- /dev/null
> > +++ b/libavformat/mnuenc.c
> > @@ -0,0 +1,64 @@
> > +/*
> > + * MNU demuxer
> > + * Copyright (c) 2012 David Girault
> > + *
> > + * This file is part of Libav.
> > + *
> > + * Libav is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * Libav is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with Libav; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > 02110-1301 USA
> > + */
> > +
> > +#include "libavutil/mathematics.h"
> > +#include "libavcodec/bytestream.h"
> > +#include "avformat.h"
> > +#include "internal.h"
> > +
> > +static int mnu_write_header(AVFormatContext *s)
> > +{
> > + AVCodecContext *ctx;
> > +
> > + if (s->nb_streams != 1) {
> > + av_log(s, AV_LOG_ERROR, "Format supports only exactly one overlay
> > stream\n");
> > + return AVERROR(EINVAL);
> > + }
> > + ctx = s->streams[0]->codec;
> > + if (ctx->codec_type != AVMEDIA_TYPE_OVERLAY || ctx->codec_id !=
> > CODEC_ID_HDMV_IGS_MENU) {
> > + av_log(s, AV_LOG_ERROR, "Currently only HDMV IGS menu is
> > supported!\n");
> > + return AVERROR(EINVAL);
> > + }
> > + return 0;
> > +}
> > +
> > +static int mnu_write_packet(AVFormatContext *s, AVPacket *pkt)
> > +{
> > + AVIOContext *pb = s->pb;
> > + avio_write(pb, "IG", 2);
> > + avio_wb32(pb, pkt->pts);
> > + avio_wb32(pb, pkt->dts);
> > + avio_write(pb, pkt->data, pkt->size);
> > + avio_flush(pb);
> > + return 0;
> > +}
> > +
> > +AVOutputFormat ff_mnu_muxer = {
> > + .name = "mnu",
> > + .long_name = NULL_IF_CONFIG_SMALL("HDMV Interactive Graphic menus
> > format"),
> > + .extensions = "mnu",
> > + .audio_codec = CODEC_ID_NONE,
> > + .video_codec = CODEC_ID_NONE,
> > + .subtitle_codec = CODEC_ID_NONE,
> > + .overlay_codec = CODEC_ID_HDMV_IGS_MENU,
> > + .write_header = mnu_write_header,
> > + .write_packet = mnu_write_packet,
> > +};
> > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> [...]
>
> --Alex
Just a little question now, what is the best way for sending the new
version of this patch? The five patches are on a same local git branch.
Is an additional patch (6/5) that fixes your concern is ok?
Is someone have some links to read?
Regards,
David
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
