From patchwork Tue Feb 7 02:11:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aaron Colwell X-Patchwork-Id: 2441 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.89.21 with SMTP id n21csp2049520vsb; Mon, 6 Feb 2017 18:19:16 -0800 (PST) X-Received: by 10.28.51.72 with SMTP id z69mr12188210wmz.38.1486433956282; Mon, 06 Feb 2017 18:19:16 -0800 (PST) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id d64si10313962wmh.24.2017.02.06.18.19.15; Mon, 06 Feb 2017 18:19:16 -0800 (PST) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@google.com; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 312F9689222; Tue, 7 Feb 2017 04:19:10 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-yb0-f174.google.com (mail-yb0-f174.google.com [209.85.213.174]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 751D9680C9A for ; Tue, 7 Feb 2017 04:19:03 +0200 (EET) Received: by mail-yb0-f174.google.com with SMTP id j82so31208611ybg.1 for ; Mon, 06 Feb 2017 18:19:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=YFMqbugkAdjVCczBQC9Hs1HX5t9HrlSEnQptwAfELgE=; b=VSA2DtEBNEFKEDMw4EBKTgpryZ3fhrRpMTnbdbdKbRjgJGe5oi7SolbgePqs6n6EMk qB48uII3fADZ1PBCTJdkMWDsD0anPy3eBQOSjI0Gd0SykpreS9FN5t4fy3i0eeQ2WjKW hFIMO66zAwl8j8tAN5ENCLZAE8KJ8ZtK7+C+tBhjaNxqzzeRlRLqNgLQqYGyHbqM60mm Ry5vi7l14xBl/hwQLWRkzrkEUxKBjIIA1iFH2XpuL3V0rDCeRPu5cuzZ8qeT2rgdSO06 F31hGSm+mUB2sE746or7nOVOb8KEzA9uqr2bbWEeGT5+oVfMKA1ZX5q1xyTdWtRHwNtn AGDw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=YFMqbugkAdjVCczBQC9Hs1HX5t9HrlSEnQptwAfELgE=; b=hOlzOnM/AeZ4RPzHi/Mw/L7zZX2rzA+AwUv9zdvCmcCmbnZ1qg1KHwq10fRWFlzofV asoE45b+91LLK8CKxyZnukgsts/oVjWEdfySsdZAYj2sLcEzmlBpE7X1rBvcvx9aioHB dbyu/WWsLw30fqiUoQSfYVjmgFMGG1diSZjtKTYuWv4jevLtwQoRv8LLx1+xzTYRB5s+ uVoltuMTPmYdHo5pCtPGh0O394GlUmBXDAbh+sps4TxZd5XtYxSgadmevPSiWh0DnRoq lhj1RxqovEWTWwLvUMVA0l77Io4P3Q1pDxlcjIGtKjSjeVcvbaFXFZ3H6Va7zBs4A2Ka i3Yw== X-Gm-Message-State: AMke39nKYRCcEwbfDfQOru2QJlUgdfuSYXsFwsnnERAYmDhHpWxb+S2pwkVPCKx4spXSaQIEWp1SmMQouUmsoU47 X-Received: by 10.37.115.215 with SMTP id o206mr9349273ybc.139.1486433474789; Mon, 06 Feb 2017 18:11:14 -0800 (PST) MIME-Version: 1.0 References: <20170207005714.GI5776@nb4> In-Reply-To: <20170207005714.GI5776@nb4> From: Aaron Colwell Date: Tue, 07 Feb 2017 02:11:03 +0000 Message-ID: To: FFmpeg development discussions and patches X-Content-Filtered-By: Mailman/MimeDel 2.1.20 Subject: Re: [FFmpeg-devel] [PATCH] mov, matroskadec : Allow matroskadec & mov to share spherical parsing logic. X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" I've attached an updated patch and modified test suite files so that the code now passes FATE on my local machine. The media files needed to be updated because they contained fields that indicated tiled-equirect. Based on the discussion in another thread, tiled-equirect is supposed to get its own enum value so technically the existing code was being more permissive than it should have been and was signalling full sphere equirect when it shouldn't have been. Full sphere equirect is actually what most people use so it seems reasonable to adjust the test files to reflect that case instead. Aaron On Mon, Feb 6, 2017 at 4:57 PM Michael Niedermayer wrote: > On Mon, Feb 06, 2017 at 09:59:58PM +0000, Aaron Colwell wrote: > > - Extracts common spherical metadata parsing logic. > > - Adds checks to enforce that only non-tiled equirect & non-padded > cubemaps > > are accepted. > > > Makefile | 5 ++- > > matroskadec.c | 39 ++++++++++++++-------------- > > mov.c | 35 ++++++++++++++----------- > > mov_spherical.c | 78 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > mov_spherical.h | 61 +++++++++++++++++++++++++++++++++++++++++++ > > 5 files changed, 182 insertions(+), 36 deletions(-) > > ebec975d2645e6cde0902b8a966545617fc420dc > 0001-mov-matroskadec-Allow-matroskadec-mov-to-share-spher.patch > > From 218dfd7ca1924bc44a168b34ec3c07051823c2dc Mon Sep 17 00:00:00 2001 > > From: Aaron Colwell > > Date: Mon, 6 Feb 2017 13:48:58 -0800 > > Subject: [PATCH] mov,matroskadec : Allow matroskadec & mov to share > spherical > > parsing logic. > > > > - Extracts common spherical metadata parsing logic. > > - Adds checks to enforce that only non-tiled equirect & non-padded > cubemaps > > are accepted. > > --- > > libavformat/Makefile | 5 +-- > > libavformat/matroskadec.c | 39 ++++++++++++----------- > > libavformat/mov.c | 35 +++++++++++--------- > > libavformat/mov_spherical.c | 78 > +++++++++++++++++++++++++++++++++++++++++++++ > > libavformat/mov_spherical.h | 61 +++++++++++++++++++++++++++++++++++ > > 5 files changed, 182 insertions(+), 36 deletions(-) > > create mode 100644 libavformat/mov_spherical.c > > create mode 100644 libavformat/mov_spherical.h > > breaks fate-h264-direct-bff > TEST h264-direct-bff > --- ./tests/ref/fate/h264-direct-bff 2017-02-06 12:12:00.827925330 +0100 > +++ tests/data/fate/h264-direct-bff 2017-02-07 01:54:38.584965163 +0100 > @@ -1,16 +0,0 @@ > -#tb 0: 1001/30000 > -#media_type 0: video > -#codec_id 0: rawvideo > -#dimensions 0: 720x484 > -#sar 0: 1/1 > -0, 0, 0, 1, 522720, 0x1ccad503 > -0, 1, 1, 1, 522720, 0xd266d6e8 > -0, 2, 2, 1, 522720, 0x535473b3 > -0, 3, 3, 1, 522720, 0xf8b53c53 > -0, 4, 4, 1, 522720, 0x4e4cc04b > -0, 5, 5, 1, 522720, 0x20ea3515 > -0, 6, 6, 1, 522720, 0xb9c67e30 > -0, 7, 7, 1, 522720, 0x03d2e35a > -0, 8, 8, 1, 522720, 0xae2e7896 > -0, 9, 9, 1, 522720, 0x6da37f41 > -0, 12, 12, 1, 522720, 0x7caf4954 > Test h264-direct-bff failed. Look at tests/data/fate/h264-direct-bff.err > for details. > make: *** [fate-h264-direct-bff] Error 1 > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Asymptotically faster algorithms should always be preferred if you have > asymptotical amounts of data > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > From a953c6fb909cfabb5d208b22d02e8904d4095734 Mon Sep 17 00:00:00 2001 From: Aaron Colwell Date: Mon, 6 Feb 2017 13:48:58 -0800 Subject: [PATCH] mov,matroskadec : Allow matroskadec & mov to share spherical parsing logic. - Extracts common spherical metadata parsing logic. - Adds checks to enforce that only non-tiled equirect & non-padded cubemaps are accepted. --- libavformat/Makefile | 5 +-- libavformat/matroskadec.c | 42 +++++++++++++----------- libavformat/mov.c | 35 +++++++++++--------- libavformat/mov_spherical.c | 78 +++++++++++++++++++++++++++++++++++++++++++++ libavformat/mov_spherical.h | 61 +++++++++++++++++++++++++++++++++++ 5 files changed, 185 insertions(+), 36 deletions(-) create mode 100644 libavformat/mov_spherical.c create mode 100644 libavformat/mov_spherical.h diff --git a/libavformat/Makefile b/libavformat/Makefile index fc2d76067b..15ec4e42b6 100644 --- a/libavformat/Makefile +++ b/libavformat/Makefile @@ -265,7 +265,8 @@ OBJS-$(CONFIG_M4V_MUXER) += rawenc.o OBJS-$(CONFIG_MATROSKA_DEMUXER) += matroskadec.o matroska.o \ rmsipr.o flac_picture.o \ oggparsevorbis.o vorbiscomment.o \ - flac_picture.o replaygain.o + flac_picture.o replaygain.o \ + mov_spherical.o OBJS-$(CONFIG_MATROSKA_MUXER) += matroskaenc.o matroska.o \ avc.o hevc.o \ flacenc_header.o avlanguage.o vorbiscomment.o wv.o \ @@ -283,7 +284,7 @@ OBJS-$(CONFIG_MLV_DEMUXER) += mlvdec.o riffdec.o OBJS-$(CONFIG_MM_DEMUXER) += mm.o OBJS-$(CONFIG_MMF_DEMUXER) += mmf.o OBJS-$(CONFIG_MMF_MUXER) += mmf.o rawenc.o -OBJS-$(CONFIG_MOV_DEMUXER) += mov.o mov_chan.o replaygain.o +OBJS-$(CONFIG_MOV_DEMUXER) += mov.o mov_chan.o mov_spherical.o replaygain.o OBJS-$(CONFIG_MOV_MUXER) += movenc.o avc.o hevc.o vpcc.o \ movenchint.o mov_chan.o rtp.o \ movenccenc.o rawutils.o diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index 7223e94b55..abb8676762 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -54,6 +54,7 @@ #include "internal.h" #include "isom.h" #include "matroska.h" +#include "mov_spherical.h" #include "oggdec.h" /* For ff_codec_get_id(). */ #include "riff.h" @@ -1909,36 +1910,39 @@ static int mkv_parse_video_color(AVStream *st, const MatroskaTrack *track) { return 0; } -static int mkv_parse_video_projection(AVStream *st, const MatroskaTrack *track) { +static int mkv_parse_video_projection(AVFormatContext *fc, AVStream *st, const MatroskaTrack *track) { AVSphericalMapping *spherical; - enum AVSphericalProjection projection; size_t spherical_size; - int ret; + int ret = 0; + AVIOContext pb; - switch (track->video.projection.type) { - case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR: - projection = AV_SPHERICAL_EQUIRECTANGULAR; - break; - case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP: - if (track->video.projection.private.size < 4) - return AVERROR_INVALIDDATA; - projection = AV_SPHERICAL_CUBEMAP; - break; - default: - return 0; - } + if (track->video.projection.type == MATROSKA_VIDEO_PROJECTION_TYPE_RECTANGULAR) + return 0; spherical = av_spherical_alloc(&spherical_size); if (!spherical) return AVERROR(ENOMEM); - spherical->projection = projection; spherical->yaw = (int32_t)(track->video.projection.yaw * (1 << 16)); spherical->pitch = (int32_t)(track->video.projection.pitch * (1 << 16)); spherical->roll = (int32_t)(track->video.projection.roll * (1 << 16)); - ret = av_stream_add_side_data(st, AV_PKT_DATA_SPHERICAL, (uint8_t *)spherical, - spherical_size); + ffio_init_context(&pb, track->video.projection.private.data, track->video.projection.private.size, + 0, NULL, NULL, NULL, NULL); + switch (track->video.projection.type) { + case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR: + ret = ff_mov_read_equi(fc, &pb, track->video.projection.private.size, spherical); + break; + case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP: + ret = ff_mov_read_cbmp(fc, &pb, track->video.projection.private.size, spherical); + break; + default: + ret = AVERROR_PATCHWELCOME; + } + + if (!ret) + ret = av_stream_add_side_data(st, AV_PKT_DATA_SPHERICAL, (uint8_t *)spherical, spherical_size); + if (ret < 0) { av_freep(&spherical); return ret; @@ -2427,7 +2431,7 @@ static int matroska_parse_tracks(AVFormatContext *s) ret = mkv_parse_video_color(st, track); if (ret < 0) return ret; - ret = mkv_parse_video_projection(st, track); + ret = mkv_parse_video_projection(matroska->ctx, st, track); if (ret < 0) return ret; } else if (track->type == MATROSKA_TRACK_TYPE_AUDIO) { diff --git a/libavformat/mov.c b/libavformat/mov.c index 6fd43a0a4e..9fdaca0a28 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -56,6 +56,7 @@ #include "libavcodec/get_bits.h" #include "id3v1.h" #include "mov_chan.h" +#include "mov_spherical.h" #include "replaygain.h" #if CONFIG_ZLIB @@ -4624,7 +4625,7 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, MOVAtom atom) int size; int32_t yaw, pitch, roll; uint32_t tag; - enum AVSphericalProjection projection; + int ret = 0; if (c->fc->nb_streams < 1) return 0; @@ -4676,32 +4677,36 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, MOVAtom atom) roll = avio_rb32(pb); size = avio_rb32(pb); - if (size > atom.size) + if (size < 8 && size > atom.size) return AVERROR_INVALIDDATA; tag = avio_rl32(pb); - avio_skip(pb, 4); /* version + flags */ + + sc->spherical = av_spherical_alloc(&sc->spherical_size); + if (!sc->spherical) + return AVERROR(ENOMEM); + + sc->spherical->yaw = yaw; + sc->spherical->pitch = pitch; + sc->spherical->roll = roll; + switch (tag) { case MKTAG('c','b','m','p'): - projection = AV_SPHERICAL_CUBEMAP; + ret = ff_mov_read_cbmp(c->fc, pb, size - 8, sc->spherical); break; case MKTAG('e','q','u','i'): - projection = AV_SPHERICAL_EQUIRECTANGULAR; + ret = ff_mov_read_equi(c->fc, pb, size - 8, sc->spherical); break; default: av_log(c->fc, AV_LOG_ERROR, "Unknown projection type\n"); - return 0; + ret = AVERROR_PATCHWELCOME; } - sc->spherical = av_spherical_alloc(&sc->spherical_size); - if (!sc->spherical) - return AVERROR(ENOMEM); - - sc->spherical->projection = projection; - - sc->spherical->yaw = yaw; - sc->spherical->pitch = pitch; - sc->spherical->roll = roll; + if (ret < 0) { + av_freep(&sc->spherical); + sc->spherical_size = 0; + return ret; + } return 0; } diff --git a/libavformat/mov_spherical.c b/libavformat/mov_spherical.c new file mode 100644 index 0000000000..a9971142bd --- /dev/null +++ b/libavformat/mov_spherical.c @@ -0,0 +1,78 @@ +/* + * Copyright (c) 2017 Aaron Colwell + * + * This file is part of FFmpeg. + * + * FFmpeg 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. + * + * FFmpeg 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 FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include "mov_spherical.h" + +int ff_mov_read_equi(AVFormatContext *fc, AVIOContext *pb, int64_t size, + AVSphericalMapping* spherical_mapping) +{ + uint32_t projection_bounds_top; + uint32_t projection_bounds_bottom; + uint32_t projection_bounds_left; + uint32_t projection_bounds_right; + + if (size == 0) { + spherical_mapping->projection = AV_SPHERICAL_EQUIRECTANGULAR; + return 0; + } + + if (size < 20) + return AVERROR_INVALIDDATA; + + avio_skip(pb, 4); /* version + flags */ + projection_bounds_top = avio_rb32(pb); + projection_bounds_bottom = avio_rb32(pb); + projection_bounds_left = avio_rb32(pb); + projection_bounds_right = avio_rb32(pb); + + if (projection_bounds_top != 0 || projection_bounds_bottom != 0 || + projection_bounds_left != 0 || projection_bounds_right != 0) { + av_log(fc, AV_LOG_ERROR, "Unsupported equirect bounds. top %u bottom %u left %u right %u\n", + projection_bounds_top, projection_bounds_bottom, + projection_bounds_left, projection_bounds_right); + return AVERROR_PATCHWELCOME; + } + + spherical_mapping->projection = AV_SPHERICAL_EQUIRECTANGULAR; + return 0; +} + +int ff_mov_read_cbmp(AVFormatContext *fc, AVIOContext *pb, int64_t size, + AVSphericalMapping* spherical_mapping) +{ + uint32_t layout; + uint32_t padding; + + if (size < 12) + return AVERROR_INVALIDDATA; + + avio_skip(pb, 4); /* version + flags */ + layout = avio_rb32(pb); + padding = avio_rb32(pb); + + if (layout != 0 || padding != 0) { + av_log(fc, AV_LOG_ERROR, "Unsupported cubemap parameters. layout %u, padding %u\n", + layout, padding); + return AVERROR_PATCHWELCOME; + } + + spherical_mapping->projection = AV_SPHERICAL_CUBEMAP; + return 0; +} diff --git a/libavformat/mov_spherical.h b/libavformat/mov_spherical.h new file mode 100644 index 0000000000..fe514a4b6c --- /dev/null +++ b/libavformat/mov_spherical.h @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2017 Aaron Colwell + * + * This file is part of FFmpeg. + * + * FFmpeg 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. + * + * FFmpeg 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 FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/** + * mov 'equi', 'cbmp' tag reading/writing. + * @author Aaron Colwell + */ + +#ifndef AVFORMAT_MOV_SPHERICAL_H +#define AVFORMAT_MOV_SPHERICAL_H + +#include +#include "libavcodec/avcodec.h" +#include "libavformat/avformat.h" +#include "libavformat/avio.h" +#include "libavutil/spherical.h" + +/** + * Parses the contents of an 'equi' box and sets the appropriate fields in spherical_mapping. + * + * @param fc AVFormatContext + * @param pb AVIOContext containing 'equi' box data. + * @param size Remaining size in the 'equi' box + * @param spherical_mapping Structure updated with parsed data. + * @return 0 if ok, or negative AVERROR code on failure + * + */ +int ff_mov_read_equi(AVFormatContext *fc, AVIOContext *pb, int64_t size, + AVSphericalMapping* spherical_mapping); + +/** + * Parses the contents of an 'cbmp' box and sets the appropriate fields in spherical_mapping. + * + * @param fc AVFormatContext + * @param pb AVIOContext containing 'cbmp' box data. + * @param size Remaining size in the 'cbmp' box + * @param spherical_mapping Structure updated with parsed data. + * @return 0 if ok, or negative AVERROR code on failure + * + */ +int ff_mov_read_cbmp(AVFormatContext *fc, AVIOContext *pb, int64_t size, + AVSphericalMapping* spherical_mapping); + +#endif /* AVFORMAT_MOV_SPHERICAL_H */ -- 2.11.0.483.g087da7b7c-goog