Le 02/06/2012 01:22, Alex Converse a écrit :
> On Fri, Jun 1, 2012 at 1:59 PM, David Girault <[email protected]> wrote:
>> 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.
>>
> This part is fine, we do have a minimum size
>
>>>> +        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).
>>
> Background on the probe. The probe buffer starts with 2048 bytes, if
> it fails to find a format with confidence it retries with twice as
> much data. It continues this retry process all the way up to 1 MB. At
> 1 MB if there is still no confident format it picks the format with
> the highest score. The max segment size seems to be 65535 so this
> should easily fit.
>
>> 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.
>>
> This seems more reasonable.
>
>> 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?
>>
> This is probably ok
>
>> 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?
>>
> The preferred way to do it is to rebase the changes into this patch,
> then format-patch the whole patchset and resend just the changed
> patches (in this case just this patch with with git send-email, use
> the message ID of something in this thread when prompted for message
> id).
>
> If you are unfamiliar with rebase, the following is good background:
> http://learn.github.com/p/rebasing.html and
> http://git-scm.com/book/en/Git-Tools-Rewriting-History#Changing-Multiple-Commit-Messages
>
> --Alex

Thanks for your advice. I've posted an update full patch in 4 parts.
See thread '[PATCH 0/4] Support for HDMV Interactive Graphics Stream'
for the last version.

Please note that I have some test samples, but all created by the same
app (a trial version).

Regards,
David

_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to