Re: [libav-devel] [PATCH] oggdec: add support for VP8 demuxing
On Tue, Dec 16, 2014 at 8:39 PM, Vittorio Giovara wrote: > From: James Almer > > Signed-off-by: Vittorio Giovara > --- > Possibly last version. > Vittorio > > Changelog | 1 + > libavformat/Makefile | 1 + > libavformat/oggdec.c | 1 + > libavformat/oggdec.h | 1 + > libavformat/oggparsevp8.c | 142 > ++ > libavformat/version.h | 4 +- > 6 files changed, 148 insertions(+), 2 deletions(-) > create mode 100644 libavformat/oggparsevp8.c This version looks ok to me and the author, so pushing later today. Vittorio ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] oggdec: add support for VP8 demuxing
On 16/12/14 3:05 PM, Vittorio Giovara wrote: > On Tue, Dec 16, 2014 at 6:10 PM, James Almer wrote: >> On 16/12/14 11:58 AM, Vittorio Giovara wrote: >>> From: James Almer >>> >>> Signed-off-by: Vittorio Giovara >>> --- >>> Dropped the sign-offs since the file was modified. >>> Addressed Anton's and James' comment. >>> Vittorio >>> >>> Changelog | 1 + >>> libavformat/Makefile | 1 + >>> libavformat/oggdec.c | 1 + >>> libavformat/oggdec.h | 1 + >>> libavformat/oggparsevp8.c | 142 >>> ++ >>> libavformat/version.h | 4 +- >>> 6 files changed, 148 insertions(+), 2 deletions(-) >>> create mode 100644 libavformat/oggparsevp8.c >> >> Please undo the change Anton requested. It was correct in the first patch. >> Check the samples from http://people.freedesktop.org/~slomo/ogg-vp8/ and see >> the framerate it reports for them. > > Thanks for the link. According to the specifications hosted there > http://people.freedesktop.org/~slomo/ogg-vp8/ogg-vp8.pdf it looks like > numerator and denominator are parsed wrong. > > your code > st->codec->width= AV_RB16(p + 8); > st->codec->height = AV_RB16(p + 10); > st->sample_aspect_ratio.num = AV_RB24(p + 12); > st->sample_aspect_ratio.den = AV_RB24(p + 15); > framerate.den = AV_RB32(p + 18); > framerate.num = AV_RB32(p + 22); > > spec code > FW 16 Stored frame width. > FH 16 Stored frame height. > PARN 24 Pixel aspect ratio numerator. > PARD 24 Pixel aspect ratio denominator. > FPSN 32 Frame rate numerator. > FPSD 32 Frame rate denominator > > So it looks like the change Anton requested was correct and the > AV_RB32 need to be inverted, unless I am missing something. James, can > you confirm for me please? Yes, that should work as well. > > On an unrelated note, would it be possible to have a fate test? > Thanks Sure, I'll send one after this is committed. ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] oggdec: add support for VP8 demuxing
On Tue, Dec 16, 2014 at 6:10 PM, James Almer wrote: > On 16/12/14 11:58 AM, Vittorio Giovara wrote: >> From: James Almer >> >> Signed-off-by: Vittorio Giovara >> --- >> Dropped the sign-offs since the file was modified. >> Addressed Anton's and James' comment. >> Vittorio >> >> Changelog | 1 + >> libavformat/Makefile | 1 + >> libavformat/oggdec.c | 1 + >> libavformat/oggdec.h | 1 + >> libavformat/oggparsevp8.c | 142 >> ++ >> libavformat/version.h | 4 +- >> 6 files changed, 148 insertions(+), 2 deletions(-) >> create mode 100644 libavformat/oggparsevp8.c > > Please undo the change Anton requested. It was correct in the first patch. > Check the samples from http://people.freedesktop.org/~slomo/ogg-vp8/ and see > the framerate it reports for them. Thanks for the link. According to the specifications hosted there http://people.freedesktop.org/~slomo/ogg-vp8/ogg-vp8.pdf it looks like numerator and denominator are parsed wrong. your code st->codec->width= AV_RB16(p + 8); st->codec->height = AV_RB16(p + 10); st->sample_aspect_ratio.num = AV_RB24(p + 12); st->sample_aspect_ratio.den = AV_RB24(p + 15); framerate.den = AV_RB32(p + 18); framerate.num = AV_RB32(p + 22); spec code FW 16 Stored frame width. FH 16 Stored frame height. PARN 24 Pixel aspect ratio numerator. PARD 24 Pixel aspect ratio denominator. FPSN 32 Frame rate numerator. FPSD 32 Frame rate denominator So it looks like the change Anton requested was correct and the AV_RB32 need to be inverted, unless I am missing something. James, can you confirm for me please? On an unrelated note, would it be possible to have a fate test? Thanks -- Vittorio ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] oggdec: add support for VP8 demuxing
On 16/12/14 11:58 AM, Vittorio Giovara wrote: > From: James Almer > > Signed-off-by: Vittorio Giovara > --- > Dropped the sign-offs since the file was modified. > Addressed Anton's and James' comment. > Vittorio > > Changelog | 1 + > libavformat/Makefile | 1 + > libavformat/oggdec.c | 1 + > libavformat/oggdec.h | 1 + > libavformat/oggparsevp8.c | 142 > ++ > libavformat/version.h | 4 +- > 6 files changed, 148 insertions(+), 2 deletions(-) > create mode 100644 libavformat/oggparsevp8.c Please undo the change Anton requested. It was correct in the first patch. Check the samples from http://people.freedesktop.org/~slomo/ogg-vp8/ and see the framerate it reports for them. ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] oggdec: add support for VP8 demuxing
Quoting Vittorio Giovara (2014-12-12 21:57:43)
> From: James Almer
>
> Signed-off-by: James Almer
> Signed-off-by: Michael Niedermayer
> Signed-off-by: Vittorio Giovara
> +static int vp8_header(AVFormatContext *s, int idx)
> +{
> +struct ogg *ogg = s->priv_data;
> +struct ogg_stream *os = ogg->streams + idx;
> +uint8_t *p = os->buf + os->pstart;
> +AVStream *st = s->streams[idx];
> +AVRational framerate;
> +
> +if (os->psize < 7 || p[0] != 0x4f)
> +return 0;
> +
> +switch (p[5]){
> +case 0x01:
> +if (os->psize < VP8_HEADER_SIZE) {
> +av_log(s, AV_LOG_ERROR, "Invalid OggVP8 header packet");
> +return AVERROR_INVALIDDATA;
> +}
> +
> +if (p[6] != 1) {
> +av_log(s, AV_LOG_WARNING,
> + "Unknown OggVP8 version %d.%d\n", p[6], p[7]);
> +return AVERROR_INVALIDDATA;
> +}
> +
> +st->codec->width= AV_RB16(p + 8);
> +st->codec->height = AV_RB16(p + 10);
> +st->sample_aspect_ratio.num = AV_RB24(p + 12);
> +st->sample_aspect_ratio.den = AV_RB24(p + 15);
> +framerate.den = AV_RB32(p + 18);
> +framerate.num = AV_RB32(p + 22);
> +
> +avpriv_set_pts_info(st, 64, framerate.num, framerate.den);
This looks reversed.
--
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] oggdec: add support for VP8 demuxing
On 12/12/14 5:57 PM, Vittorio Giovara wrote:
> From: James Almer
>
> Signed-off-by: James Almer
> Signed-off-by: Michael Niedermayer
> Signed-off-by: Vittorio Giovara
> ---
> Changelog | 1 +
> libavformat/Makefile | 1 +
> libavformat/oggdec.c | 1 +
> libavformat/oggdec.h | 1 +
> libavformat/oggparsevp8.c | 142
> ++
> libavformat/version.h | 4 +-
> 6 files changed, 148 insertions(+), 2 deletions(-)
> create mode 100644 libavformat/oggparsevp8.c
>
[...]
> diff --git a/libavformat/oggparsevp8.c b/libavformat/oggparsevp8.c
> new file mode 100644
> index 000..1256bfe
> --- /dev/null
> +++ b/libavformat/oggparsevp8.c
> @@ -0,0 +1,142 @@
> +/*
> + * On2 VP8 parser for Ogg
> + * Copyright (C) 2013 James Almer
> + *
> + * 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/intreadwrite.h"
> +
> +#include "avformat.h"
> +#include "internal.h"
> +#include "oggdec.h"
> +
> +#define VP8_HEADER_SIZE 26
> +
> +static int vp8_header(AVFormatContext *s, int idx)
> +{
> +struct ogg *ogg = s->priv_data;
> +struct ogg_stream *os = ogg->streams + idx;
> +uint8_t *p = os->buf + os->pstart;
> +AVStream *st = s->streams[idx];
> +AVRational framerate;
> +
> +if (os->psize < 7 || p[0] != 0x4f)
> +return 0;
> +
> +switch (p[5]){
> +case 0x01:
> +if (os->psize < VP8_HEADER_SIZE) {
> +av_log(s, AV_LOG_ERROR, "Invalid OggVP8 header packet");
> +return AVERROR_INVALIDDATA;
> +}
> +
> +if (p[6] != 1) {
> +av_log(s, AV_LOG_WARNING,
> + "Unknown OggVP8 version %d.%d\n", p[6], p[7]);
> +return AVERROR_INVALIDDATA;
> +}
> +
> +st->codec->width= AV_RB16(p + 8);
> +st->codec->height = AV_RB16(p + 10);
> +st->sample_aspect_ratio.num = AV_RB24(p + 12);
> +st->sample_aspect_ratio.den = AV_RB24(p + 15);
> +framerate.den = AV_RB32(p + 18);
> +framerate.num = AV_RB32(p + 22);
> +
> +avpriv_set_pts_info(st, 64, framerate.num, framerate.den);
> +st->codec->codec_type = AVMEDIA_TYPE_VIDEO;
> +st->codec->codec_id = AV_CODEC_ID_VP8;
> +st->need_parsing = AVSTREAM_PARSE_HEADERS;
> +break;
> +case 0x02:
> +if (p[6] != 0x20)
> +return AVERROR_INVALIDDATA;
> +ff_vorbis_comment(s, &st->metadata, p + 7, os->psize - 7, 1);
ff_vorbis_stream_comment() for consistency with the other parsers.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] oggdec: add support for VP8 demuxing
On 12/12/14 21:57, Vittorio Giovara wrote: From: James Almer Signed-off-by: James Almer Signed-off-by: Michael Niedermayer Signed-off-by: Vittorio Giovara --- Changelog | 1 + libavformat/Makefile | 1 + libavformat/oggdec.c | 1 + libavformat/oggdec.h | 1 + libavformat/oggparsevp8.c | 142 ++ libavformat/version.h | 4 +- 6 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 libavformat/oggparsevp8.c diff --git a/Changelog b/Changelog index 6af2e8a..581f377 100644 --- a/Changelog +++ b/Changelog @@ -7,6 +7,7 @@ version : - avplay now exits by default at the end of playback - XCB-based screen-grabber - creating DASH compatible fragmented MP4, MPEG-DASH segmenting muxer +- VP8 in Ogg demuxing Looks fine to me. ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
