Re: [FFmpeg-devel] [PATCH][RFC] avutil/spherical: add a flag to signal tiled/padded projections
On Wed, Mar 29, 2017 at 8:01 PM, James Almer wrote: > A new field is added to AVSphericalMapping for this purpose, > and is used by both Equirectangular and Cubemap projections. > This is a replacement for duplicate projection enums like > AV_SPHERICAL_EQUIRECTANGULAR_TILE, which are therefore > removed. > > Signed-off-by: James Almer > --- > This patch depends on the av_spherical_projection_name() patchset for > simplicity purposes. > > Ok, this is an RFC mainly because of the API/ABI break it represents. > The AV_SPHERICAL_EQUIRECTANGULAR_TILE projection is a month and a half > old and not present in any release, plus a major bump is queued as part > of the merges, so i personally think this change is acceptable as is for > such an niche and recent feature. > If not then i can deprecate said projection enum value instead and keep > the current muxer functionality for it for a while. It will need a lot > of preprocessor guards, though. > > The reason for this change is that eventually, a new projection enum > for padded cubemap may be suggested with the same arguments as the ones > used to introduce the one for tiled equirectangular. Then if any new > real projections are added, we'd could end up with duplicate enums for > them as well, when setting a single shared flag would be enough. > Stereo3D avoided a lot of duplicate types with the inverted flag, so i > figured the same should be done here. > > Improved doxy and/or flag name is extremely welcome (Read: needed). I'm against this idea because of explanations given in other threads. The stereo3d case is different because it's a property that can be applied to all types, while the cropped/padded feature applies to current existing projections, not the future ones. Additionally there is nothing wrong with having more enum values, since it is likely that future cubemaps layouts will have a different enum value, and not another field to check. Regardless of the outcome of the discussion, please keep the API aligned for users' sake. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] spherical: Change types of bounding and pad to uint32_t
These values are defined to be 32bit in the specification,
so it makes more sense to store them as fixed width.
Based on a patch by Micahel Niedermayer .
Signed-off-by: Vittorio Giovara
---
Hi,
this is the version which applies cleanly to master and contains
changes requested by James.
Please CC.
Vittorio
libavformat/dump.c| 2 +-
libavformat/matroskadec.c | 7 +++
libavformat/mov.c | 8 +++-
libavutil/spherical.h | 10 +-
4 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/libavformat/dump.c b/libavformat/dump.c
index 505d572301..3e6218303d 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -375,7 +375,7 @@ static void dump_spherical(void *ctx, AVCodecParameters
*par, AVPacketSideData *
&l, &t, &r, &b);
av_log(ctx, AV_LOG_INFO, "[%zu, %zu, %zu, %zu] ", l, t, r, b);
} else if (spherical->projection == AV_SPHERICAL_CUBEMAP) {
-av_log(ctx, AV_LOG_INFO, "[pad %zu] ", spherical->padding);
+av_log(ctx, AV_LOG_INFO, "[pad %"PRIu32"] ", spherical->padding);
}
}
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index fdb23ab05e..bad034b770 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1913,8 +1913,8 @@ static int mkv_parse_video_projection(AVStream *st, const
MatroskaTrack *track)
AVSphericalMapping *spherical;
enum AVSphericalProjection projection;
size_t spherical_size;
-size_t l = 0, t = 0, r = 0, b = 0;
-size_t padding = 0;
+uint32_t l = 0, t = 0, r = 0, b = 0;
+uint32_t padding = 0;
int ret;
GetByteContext gb;
@@ -1939,8 +1939,7 @@ static int mkv_parse_video_projection(AVStream *st, const
MatroskaTrack *track)
if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
av_log(NULL, AV_LOG_ERROR,
"Invalid bounding rectangle coordinates "
- "%"SIZE_SPECIFIER",%"SIZE_SPECIFIER","
- "%"SIZE_SPECIFIER",%"SIZE_SPECIFIER"\n",
+ "%"PRIu32",%"PRIu32",%"PRIu32",%"PRIu32"\n",
l, t, r, b);
return AVERROR_INVALIDDATA;
}
diff --git a/libavformat/mov.c b/libavformat/mov.c
index d5c3949050..5e7be49563 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4637,9 +4637,8 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
MOVStreamContext *sc;
int size, layout;
int32_t yaw, pitch, roll;
-size_t l = 0, t = 0, r = 0, b = 0;
-size_t padding = 0;
-uint32_t tag;
+uint32_t l = 0, t = 0, r = 0, b = 0;
+uint32_t tag, padding = 0;
enum AVSphericalProjection projection;
if (c->fc->nb_streams < 1)
@@ -4717,8 +4716,7 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
av_log(c->fc, AV_LOG_ERROR,
"Invalid bounding rectangle coordinates %"SIZE_SPECIFIER","
- "%"SIZE_SPECIFIER",%"SIZE_SPECIFIER",%"SIZE_SPECIFIER"\n",
- l, t, r, b);
+ "%"PRIu32",%"PRIu32",%"PRIu32",%"PRIu32"\n", l, t, r, b);
return AVERROR_INVALIDDATA;
}
diff --git a/libavutil/spherical.h b/libavutil/spherical.h
index db9bdc0be5..ff1922ade7 100644
--- a/libavutil/spherical.h
+++ b/libavutil/spherical.h
@@ -164,10 +164,10 @@ typedef struct AVSphericalMapping {
* projection type (@ref AV_SPHERICAL_EQUIRECTANGULAR_TILE),
* and should be ignored in all other cases.
*/
-size_t bound_left; ///< Distance from the left edge
-size_t bound_top;///< Distance from the top edge
-size_t bound_right; ///< Distance from the right edge
-size_t bound_bottom; ///< Distance from the bottom edge
+uint32_t bound_left; ///< Distance from the left edge
+uint32_t bound_top;///< Distance from the top edge
+uint32_t bound_right; ///< Distance from the right edge
+uint32_t bound_bottom; ///< Distance from the bottom edge
/**
* @}
*/
@@ -179,7 +179,7 @@ typedef struct AVSphericalMapping {
* (@ref AV_SPHERICAL_CUBEMAP), and should be ignored in all other
* cases.
*/
-size_t padding;
+uint32_t padding;
} AVSphericalMapping;
/**
--
2.12.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] fate: Do not report side data size
On Tue, Mar 7, 2017 at 7:17 PM, Vittorio Giovara wrote: > This should address the mismatch between different archs > Please CC. > -- > Vittorio ping Please CC. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] spherical: Change types of bounding and pad to uint32_t
On Thu, Mar 16, 2017 at 4:16 PM, Lou Logan wrote: > On Wed, 15 Mar 2017 10:22:23 +0100 > Carl Eugen Hoyos wrote: > >> Sorry if I miss something but could you clarify who the author of this >> patch is? > > CC'ing as Vittorio is not subscribed to the mailing list. Not that you > should have to know that as there was no CC request in this particular > message. Hi, I'm not sure I understand the question. I wrote the patch, and I'll mention it's based on one from Michael. Is there some problem with it? Thanks for the CC. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavu/spherical: Make first parameter const
On Thu, Mar 16, 2017 at 4:26 PM, James Almer wrote: > On 3/16/2017 5:23 PM, Carl Eugen Hoyos wrote: >> Hi! >> >> Attached patch silences a gcc warning when compiling ffprobe. >> >> Please comment, Carl Eugen >> >> >> 0001-lavu-spherical-Make-AVSphericalMapping-pointer-param.patch >> >> >> From 5040ee6f7dd43c8d66a241aa0376839916cd3d9f Mon Sep 17 00:00:00 2001 >> From: Carl Eugen Hoyos >> Date: Thu, 16 Mar 2017 21:19:48 +0100 >> Subject: [PATCH] lavu/spherical: Make AVSphericalMapping pointer parameter >> const. >> >> Reflects the actual code and silences a gcc warning: >> ffprobe.c:1797:42: warning: passing argument 1 of 'av_spherical_tile_bounds' >> discards 'const' qualifier from pointer target type >> --- >> libavutil/spherical.c |2 +- >> libavutil/spherical.h |2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/libavutil/spherical.c b/libavutil/spherical.c >> index 0ca2dd3..f0b6221 100644 >> --- a/libavutil/spherical.c >> +++ b/libavutil/spherical.c >> @@ -33,7 +33,7 @@ AVSphericalMapping *av_spherical_alloc(size_t *size) >> return spherical; >> } >> >> -void av_spherical_tile_bounds(AVSphericalMapping *map, >> +void av_spherical_tile_bounds(const AVSphericalMapping *map, >>size_t width, size_t height, >>size_t *left, size_t *top, >>size_t *right, size_t *bottom) >> diff --git a/libavutil/spherical.h b/libavutil/spherical.h >> index db9bdc0..f4e0d60 100644 >> --- a/libavutil/spherical.h >> +++ b/libavutil/spherical.h >> @@ -202,7 +202,7 @@ AVSphericalMapping *av_spherical_alloc(size_t *size); >> * @param right Pixels from the right edge. >> * @param bottom Pixels from the bottom edge. >> */ >> -void av_spherical_tile_bounds(AVSphericalMapping *map, >> +void av_spherical_tile_bounds(const AVSphericalMapping *map, >>size_t width, size_t height, >>size_t *left, size_t *top, >>size_t *right, size_t *bottom); >> -- 1.7.10.4 >> > > LGTM. The AVSphericalMapping struct is not modified by the function > so it should have been const since the beginning. > lgtm too -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] spherical: Change types of bounding and pad to uint32_t
On Wed, Mar 15, 2017 at 11:31 PM, James Almer wrote:
> On 3/15/2017 6:39 PM, Vittorio Giovara wrote:
>> These values are defined to be 32bit in the specification,
>> so it makes more sense to store them as fixed width.
>>
>> Signed-off-by: Vittorio Giovara
>> ---
>> Updated commit message, changed a couple of internal types.
>> Please CC or I can't see replies.
>> Vittorio
>
> Mention it's based on a patch by Michael Niedermayer.
>
>>
>> libavformat/dump.c| 2 +-
>> libavformat/matroskadec.c | 6 +++---
>> libavformat/mov.c | 7 +++
>> libavutil/spherical.h | 10 +-
>> 4 files changed, 12 insertions(+), 13 deletions(-)
>>
>> diff --git a/libavformat/dump.c b/libavformat/dump.c
>> index 7514aee7ac..c56895628d 100644
>> --- a/libavformat/dump.c
>> +++ b/libavformat/dump.c
>> @@ -339,7 +339,7 @@ static void dump_spherical(void *ctx, AVCodecParameters
>> *par, AVPacketSideData *
>> &l, &t, &r, &b);
>> av_log(ctx, AV_LOG_INFO, "[%zu, %zu, %zu, %zu] ", l, t, r, b);
>> } else if (spherical->projection == AV_SPHERICAL_CUBEMAP) {
>> -av_log(ctx, AV_LOG_INFO, "[pad %zu] ", spherical->padding);
>> +av_log(ctx, AV_LOG_INFO, "[pad %"PRIu32"] ", spherical->padding);
>> }
>> }
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 4fbf4b9a96..c6e1a190a8 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -1601,8 +1601,8 @@ static int mkv_parse_video_projection(AVStream *st,
>> const MatroskaTrack *track)
>> AVSphericalMapping *spherical;
>> enum AVSphericalProjection projection;
>> size_t spherical_size;
>> -size_t l = 0, t = 0, r = 0, b = 0;
>> -size_t padding = 0;
>> +uint32_t l = 0, t = 0, r = 0, b = 0;
>> +uint32_t padding = 0;
>> int ret;
>> GetByteContext gb;
>>
>> @@ -1627,7 +1627,7 @@ static int mkv_parse_video_projection(AVStream *st,
>> const MatroskaTrack *track)
>> if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
>> av_log(NULL, AV_LOG_ERROR,
>> "Invalid bounding rectangle coordinates "
>> - "%zu,%zu,%zu,%zu\n", l, t, r, b);
>> + "%"PRIu32",%"PRIu32",%"PRIu32",%"PRIu32"\n", l, t,
>> r, b);
>
> This chunk doesn't apply.
>
>> return AVERROR_INVALIDDATA;
>> }
>> } else if (track->video.projection.private.size != 0) {
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index c6e7a38398..1c1857eaf9 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -3237,9 +3237,8 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext
>> *pb, MOVAtom atom)
>> MOVStreamContext *sc;
>> int size, version, layout;
>> int32_t yaw, pitch, roll;
>> -size_t l = 0, t = 0, r = 0, b = 0;
>> -size_t padding = 0;
>> -uint32_t tag;
>> +uint32_t l = 0, t = 0, r = 0, b = 0;
>> +uint32_t tag, padding = 0;
>> enum AVSphericalProjection projection;
>>
>> if (c->fc->nb_streams < 1)
>> @@ -3335,7 +3334,7 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext
>> *pb, MOVAtom atom)
>> if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
>> av_log(c->fc, AV_LOG_ERROR,
>> "Invalid bounding rectangle coordinates "
>> - "%zu,%zu,%zu,%zu\n", l, t, r, b);
>> + "%"PRIu32",%"PRIu32",%"PRIu32",%"PRIu32"\n", l, t, r, b);
>
> Same.
>
>> return AVERROR_INVALIDDATA;
>> }
>>
>> diff --git a/libavutil/spherical.h b/libavutil/spherical.h
>> index e7fc1d69fb..fd662cf676 100644
>> --- a/libavutil/spherical.h
>> +++ b/libavutil/spherical.h
>> @@ -164,10 +164,10 @@ typedef struct AVSphericalMapping {
>> * projection type (@ref AV_SPHERICAL_EQUIRECTANGULAR_TILE),
>> * and should be ignored in all other cases.
>> */
>> -size_t bound_left; ///< Distance from the left edge
>> -size_t bound_top;///< Distance from the top edge
>> -size_t bound_right; ///< Distance from the right edge
>>
[FFmpeg-devel] [PATCH] spherical: Change types of bounding and pad to uint32_t
These values are defined to be 32bit in the specification,
so it makes more sense to store them as fixed width.
Signed-off-by: Vittorio Giovara
---
Updated commit message, changed a couple of internal types.
Please CC or I can't see replies.
Vittorio
libavformat/dump.c| 2 +-
libavformat/matroskadec.c | 6 +++---
libavformat/mov.c | 7 +++
libavutil/spherical.h | 10 +-
4 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/libavformat/dump.c b/libavformat/dump.c
index 7514aee7ac..c56895628d 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -339,7 +339,7 @@ static void dump_spherical(void *ctx, AVCodecParameters
*par, AVPacketSideData *
&l, &t, &r, &b);
av_log(ctx, AV_LOG_INFO, "[%zu, %zu, %zu, %zu] ", l, t, r, b);
} else if (spherical->projection == AV_SPHERICAL_CUBEMAP) {
-av_log(ctx, AV_LOG_INFO, "[pad %zu] ", spherical->padding);
+av_log(ctx, AV_LOG_INFO, "[pad %"PRIu32"] ", spherical->padding);
}
}
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 4fbf4b9a96..c6e1a190a8 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1601,8 +1601,8 @@ static int mkv_parse_video_projection(AVStream *st, const
MatroskaTrack *track)
AVSphericalMapping *spherical;
enum AVSphericalProjection projection;
size_t spherical_size;
-size_t l = 0, t = 0, r = 0, b = 0;
-size_t padding = 0;
+uint32_t l = 0, t = 0, r = 0, b = 0;
+uint32_t padding = 0;
int ret;
GetByteContext gb;
@@ -1627,7 +1627,7 @@ static int mkv_parse_video_projection(AVStream *st, const
MatroskaTrack *track)
if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
av_log(NULL, AV_LOG_ERROR,
"Invalid bounding rectangle coordinates "
- "%zu,%zu,%zu,%zu\n", l, t, r, b);
+ "%"PRIu32",%"PRIu32",%"PRIu32",%"PRIu32"\n", l, t, r,
b);
return AVERROR_INVALIDDATA;
}
} else if (track->video.projection.private.size != 0) {
diff --git a/libavformat/mov.c b/libavformat/mov.c
index c6e7a38398..1c1857eaf9 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -3237,9 +3237,8 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
MOVStreamContext *sc;
int size, version, layout;
int32_t yaw, pitch, roll;
-size_t l = 0, t = 0, r = 0, b = 0;
-size_t padding = 0;
-uint32_t tag;
+uint32_t l = 0, t = 0, r = 0, b = 0;
+uint32_t tag, padding = 0;
enum AVSphericalProjection projection;
if (c->fc->nb_streams < 1)
@@ -3335,7 +3334,7 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
av_log(c->fc, AV_LOG_ERROR,
"Invalid bounding rectangle coordinates "
- "%zu,%zu,%zu,%zu\n", l, t, r, b);
+ "%"PRIu32",%"PRIu32",%"PRIu32",%"PRIu32"\n", l, t, r, b);
return AVERROR_INVALIDDATA;
}
diff --git a/libavutil/spherical.h b/libavutil/spherical.h
index e7fc1d69fb..fd662cf676 100644
--- a/libavutil/spherical.h
+++ b/libavutil/spherical.h
@@ -164,10 +164,10 @@ typedef struct AVSphericalMapping {
* projection type (@ref AV_SPHERICAL_EQUIRECTANGULAR_TILE),
* and should be ignored in all other cases.
*/
-size_t bound_left; ///< Distance from the left edge
-size_t bound_top;///< Distance from the top edge
-size_t bound_right; ///< Distance from the right edge
-size_t bound_bottom; ///< Distance from the bottom edge
+uint32_t bound_left; ///< Distance from the left edge
+uint32_t bound_top;///< Distance from the top edge
+uint32_t bound_right; ///< Distance from the right edge
+uint32_t bound_bottom; ///< Distance from the bottom edge
/**
* @}
*/
@@ -179,7 +179,7 @@ typedef struct AVSphericalMapping {
* (@ref AV_SPHERICAL_CUBEMAP), and should be ignored in all other
* cases.
*/
-size_t padding;
+uint32_t padding;
} AVSphericalMapping;
/**
--
2.12.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] spherical: Change types of bounding and pad to uint32_t
These types better reflect the ones described in the specification and
avoid any platform-specific implementation issues.
Signed-off-by: Vittorio Giovara
---
libavformat/dump.c| 2 +-
libavutil/spherical.h | 10 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/libavformat/dump.c b/libavformat/dump.c
index 7514aee7ac..c56895628d 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -339,7 +339,7 @@ static void dump_spherical(void *ctx, AVCodecParameters
*par, AVPacketSideData *
&l, &t, &r, &b);
av_log(ctx, AV_LOG_INFO, "[%zu, %zu, %zu, %zu] ", l, t, r, b);
} else if (spherical->projection == AV_SPHERICAL_CUBEMAP) {
-av_log(ctx, AV_LOG_INFO, "[pad %zu] ", spherical->padding);
+av_log(ctx, AV_LOG_INFO, "[pad %"PRIu32"] ", spherical->padding);
}
}
diff --git a/libavutil/spherical.h b/libavutil/spherical.h
index e7fc1d69fb..fd662cf676 100644
--- a/libavutil/spherical.h
+++ b/libavutil/spherical.h
@@ -164,10 +164,10 @@ typedef struct AVSphericalMapping {
* projection type (@ref AV_SPHERICAL_EQUIRECTANGULAR_TILE),
* and should be ignored in all other cases.
*/
-size_t bound_left; ///< Distance from the left edge
-size_t bound_top;///< Distance from the top edge
-size_t bound_right; ///< Distance from the right edge
-size_t bound_bottom; ///< Distance from the bottom edge
+uint32_t bound_left; ///< Distance from the left edge
+uint32_t bound_top;///< Distance from the top edge
+uint32_t bound_right; ///< Distance from the right edge
+uint32_t bound_bottom; ///< Distance from the bottom edge
/**
* @}
*/
@@ -179,7 +179,7 @@ typedef struct AVSphericalMapping {
* (@ref AV_SPHERICAL_CUBEMAP), and should be ignored in all other
* cases.
*/
-size_t padding;
+uint32_t padding;
} AVSphericalMapping;
/**
--
2.12.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Change type of spherical stuff to uint32_t
On Tue, Mar 14, 2017 at 5:27 PM, James Almer wrote: > On 3/14/2017 6:16 PM, Vittorio Giovara wrote: >> On Tue, Mar 14, 2017 at 4:41 PM, James Almer wrote: >>> On 3/14/2017 4:46 PM, Vittorio Giovara wrote: >>>> On Tue, Mar 14, 2017 at 12:12 PM, James Almer wrote: >>>>> On 3/14/2017 12:37 PM, Vittorio Giovara wrote: >>>>>> On Sun, Mar 12, 2017 at 11:21 PM, Michael Niedermayer >>>>>> wrote: >>>>>>> On Sun, Mar 12, 2017 at 08:54:07PM -0400, Ronald S. Bultje wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> On Sun, Mar 12, 2017 at 7:32 PM, James Almer wrote: >>>>>>>> >>>>>>>>> On 3/12/2017 6:16 PM, Michael Niedermayer wrote: >>>>>>>>>> Using the same type across platforms is more robust and avoids >>>>>>>>>> platform >>>>>>>>>> specific issues from differences in range. >>>>>> >>>>>> I still think you are curing the symptom rather than the illness. >>>>>> >>>>>> Besides, you can't just change types on a whim, you should wait for >>>>>> the major bump (if at all). >>>>> >>>>> As mentioned by Hendrik, it's only six days old and not part of any >>>>> release, so it can be changed just fine. >>>> >>>> Not really, but I'm not here to discuss policies. >>> >>> Maybe not in libav, but it is here. Being stuck with a very recent bad >>> change for no reason is not a sane choice. If it was a month old then >>> we're talking. >>> Releases are made precisely to freeze the API/ABI for distros and the >>> reason why this change either goes in now or doesn't go in at all. >>> >>>> >>>>>>>>>> Also fixed point integers are on a semantical level not size_t >>>>>> >>>>>> This is only theoretical, >>>>> >>>>> You specifically wrote the API to have the fields store 0.32 fixed point >>>>> values. Why you choose size_t for a field that's meant to store exactly >>>>> 32 bits is the question. >>>> >>>> I choose size_t because it represented a `size` as the name implies, >>>> and since it is very closely related to "cropping rectangle" I picked >>>> the types of the fields that were in use in AVFrame: >>>> https://git.libav.org/?p=libav.git;a=blob;f=libavutil/frame.h;h=f9ffb5bbbf852de4728442a3940c6f2ddb752ecd;hb=HEAD#l385 >>> >>> The size of an object in memory, not anything related to the dimensions >>> of a video. Is the latter supposed to be system dependent now? >>> >>> And unlike those AVFrames fields which are pixel values, these fields >>> are *explicitly* meant to store exactly 32 bit long, 0.32 fixed point >>> values. Anything other than uint32_t or int32_t, types defined in the >>> standard for this exact scenario, makes no sense. >>> >>>> >>>>> Afaik, size_t could even be 16 bit in some systems. FreeDOS is probably >>>>> one, and we supposedly support it. Libav does too, so maybe this change >>>>> should be done in both projects. >>>> >>>> I'm pretty sure both projects will fail to build on 16 bits platform. >>>> Unless you find a very very smart compiler that will ignore all large >>>> constants not fitting into int, all out of bounds shifts, tables not >>>> fitting into even 1MB and such. Then it will probably crash somewhere >>>> anyway. Someone mentioned me that this would be similar to a certain >>>> MPEG reference decoder from Fraunhofer that does not compile right in >>>> 64-bit mode and does not run by default because some of its functions >>>> require >10MB of stack. >>> >>> wbs on irc confirmed those targets are 32 bits as far as we support them, >>> so apparently not a problem for us after all. >>> >>>> >>>>>> and, since we're talking about semantics, >>>>>> you're breaking ABI by using sizeof(AVSphericalMapping) outside of >>>>>> libavutil. >>>>> >>>>> Well, you're the one that introduced the only current sizeof() check >>>>> outside of libavutil, in both projects. >>>> >>>> Are you sure? >>> >>> I mean the
Re: [FFmpeg-devel] [PATCH] Change type of spherical stuff to uint32_t
On Tue, Mar 14, 2017 at 5:27 PM, James Almer wrote: > On 3/14/2017 6:16 PM, Vittorio Giovara wrote: >> On Tue, Mar 14, 2017 at 4:41 PM, James Almer wrote: >>> On 3/14/2017 4:46 PM, Vittorio Giovara wrote: >>>> On Tue, Mar 14, 2017 at 12:12 PM, James Almer wrote: >>>>> On 3/14/2017 12:37 PM, Vittorio Giovara wrote: >>>>>> On Sun, Mar 12, 2017 at 11:21 PM, Michael Niedermayer >>>>>> wrote: >>>>>>> On Sun, Mar 12, 2017 at 08:54:07PM -0400, Ronald S. Bultje wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> On Sun, Mar 12, 2017 at 7:32 PM, James Almer wrote: >>>>>>>> >>>>>>>>> On 3/12/2017 6:16 PM, Michael Niedermayer wrote: >>>>>>>>>> Using the same type across platforms is more robust and avoids >>>>>>>>>> platform >>>>>>>>>> specific issues from differences in range. >>>>>> >>>>>> I still think you are curing the symptom rather than the illness. >>>>>> >>>>>> Besides, you can't just change types on a whim, you should wait for >>>>>> the major bump (if at all). >>>>> >>>>> As mentioned by Hendrik, it's only six days old and not part of any >>>>> release, so it can be changed just fine. >>>> >>>> Not really, but I'm not here to discuss policies. >>> >>> Maybe not in libav, but it is here. Being stuck with a very recent bad >>> change for no reason is not a sane choice. If it was a month old then >>> we're talking. >>> Releases are made precisely to freeze the API/ABI for distros and the >>> reason why this change either goes in now or doesn't go in at all. >>> >>>> >>>>>>>>>> Also fixed point integers are on a semantical level not size_t >>>>>> >>>>>> This is only theoretical, >>>>> >>>>> You specifically wrote the API to have the fields store 0.32 fixed point >>>>> values. Why you choose size_t for a field that's meant to store exactly >>>>> 32 bits is the question. >>>> >>>> I choose size_t because it represented a `size` as the name implies, >>>> and since it is very closely related to "cropping rectangle" I picked >>>> the types of the fields that were in use in AVFrame: >>>> https://git.libav.org/?p=libav.git;a=blob;f=libavutil/frame.h;h=f9ffb5bbbf852de4728442a3940c6f2ddb752ecd;hb=HEAD#l385 >>> >>> The size of an object in memory, not anything related to the dimensions >>> of a video. Is the latter supposed to be system dependent now? >>> >>> And unlike those AVFrames fields which are pixel values, these fields >>> are *explicitly* meant to store exactly 32 bit long, 0.32 fixed point >>> values. Anything other than uint32_t or int32_t, types defined in the >>> standard for this exact scenario, makes no sense. >>> >>>> >>>>> Afaik, size_t could even be 16 bit in some systems. FreeDOS is probably >>>>> one, and we supposedly support it. Libav does too, so maybe this change >>>>> should be done in both projects. >>>> >>>> I'm pretty sure both projects will fail to build on 16 bits platform. >>>> Unless you find a very very smart compiler that will ignore all large >>>> constants not fitting into int, all out of bounds shifts, tables not >>>> fitting into even 1MB and such. Then it will probably crash somewhere >>>> anyway. Someone mentioned me that this would be similar to a certain >>>> MPEG reference decoder from Fraunhofer that does not compile right in >>>> 64-bit mode and does not run by default because some of its functions >>>> require >10MB of stack. >>> >>> wbs on irc confirmed those targets are 32 bits as far as we support them, >>> so apparently not a problem for us after all. >>> >>>> >>>>>> and, since we're talking about semantics, >>>>>> you're breaking ABI by using sizeof(AVSphericalMapping) outside of >>>>>> libavutil. >>>>> >>>>> Well, you're the one that introduced the only current sizeof() check >>>>> outside of libavutil, in both projects. >>>> >>>> Are you sure? >>> >>> I mean the one related to AVSphericalMapping. So yes, you introduced >>> that ABI breaking check at the same time you introduced the relevant >>> struct. >>> >>> The rest are of course someone else's fault. >>> >>>> There are several invalid sizeof() uses of things that >>>> shouldn't be size-able -- this one is noticeable only because the test >>>> is printing different things. Since you're mentioning me directly, >>>> please remember that I also proposed the right fix for this bug, that >>>> is to remove printing the size of structures from ffprobe. >>> >>> That change will probably go in as well. >> >> If that change goes in, I withdraw any objection on this patch. > > But will you try to apply this patch on libav's side as well? wm4's > concern is the divergence this will create between the two projects. Yes that is a valid concern, I'll forward this patch to libav-devel and move the discussion there. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Change type of spherical stuff to uint32_t
On Tue, Mar 14, 2017 at 4:41 PM, James Almer wrote: > On 3/14/2017 4:46 PM, Vittorio Giovara wrote: >> On Tue, Mar 14, 2017 at 12:12 PM, James Almer wrote: >>> On 3/14/2017 12:37 PM, Vittorio Giovara wrote: >>>> On Sun, Mar 12, 2017 at 11:21 PM, Michael Niedermayer >>>> wrote: >>>>> On Sun, Mar 12, 2017 at 08:54:07PM -0400, Ronald S. Bultje wrote: >>>>>> Hi, >>>>>> >>>>>> On Sun, Mar 12, 2017 at 7:32 PM, James Almer wrote: >>>>>> >>>>>>> On 3/12/2017 6:16 PM, Michael Niedermayer wrote: >>>>>>>> Using the same type across platforms is more robust and avoids platform >>>>>>>> specific issues from differences in range. >>>> >>>> I still think you are curing the symptom rather than the illness. >>>> >>>> Besides, you can't just change types on a whim, you should wait for >>>> the major bump (if at all). >>> >>> As mentioned by Hendrik, it's only six days old and not part of any >>> release, so it can be changed just fine. >> >> Not really, but I'm not here to discuss policies. > > Maybe not in libav, but it is here. Being stuck with a very recent bad > change for no reason is not a sane choice. If it was a month old then > we're talking. > Releases are made precisely to freeze the API/ABI for distros and the > reason why this change either goes in now or doesn't go in at all. > >> >>>>>>>> Also fixed point integers are on a semantical level not size_t >>>> >>>> This is only theoretical, >>> >>> You specifically wrote the API to have the fields store 0.32 fixed point >>> values. Why you choose size_t for a field that's meant to store exactly >>> 32 bits is the question. >> >> I choose size_t because it represented a `size` as the name implies, >> and since it is very closely related to "cropping rectangle" I picked >> the types of the fields that were in use in AVFrame: >> https://git.libav.org/?p=libav.git;a=blob;f=libavutil/frame.h;h=f9ffb5bbbf852de4728442a3940c6f2ddb752ecd;hb=HEAD#l385 > > The size of an object in memory, not anything related to the dimensions > of a video. Is the latter supposed to be system dependent now? > > And unlike those AVFrames fields which are pixel values, these fields > are *explicitly* meant to store exactly 32 bit long, 0.32 fixed point > values. Anything other than uint32_t or int32_t, types defined in the > standard for this exact scenario, makes no sense. > >> >>> Afaik, size_t could even be 16 bit in some systems. FreeDOS is probably >>> one, and we supposedly support it. Libav does too, so maybe this change >>> should be done in both projects. >> >> I'm pretty sure both projects will fail to build on 16 bits platform. >> Unless you find a very very smart compiler that will ignore all large >> constants not fitting into int, all out of bounds shifts, tables not >> fitting into even 1MB and such. Then it will probably crash somewhere >> anyway. Someone mentioned me that this would be similar to a certain >> MPEG reference decoder from Fraunhofer that does not compile right in >> 64-bit mode and does not run by default because some of its functions >> require >10MB of stack. > > wbs on irc confirmed those targets are 32 bits as far as we support them, > so apparently not a problem for us after all. > >> >>>> and, since we're talking about semantics, >>>> you're breaking ABI by using sizeof(AVSphericalMapping) outside of >>>> libavutil. >>> >>> Well, you're the one that introduced the only current sizeof() check >>> outside of libavutil, in both projects. >> >> Are you sure? > > I mean the one related to AVSphericalMapping. So yes, you introduced > that ABI breaking check at the same time you introduced the relevant > struct. > > The rest are of course someone else's fault. > >> There are several invalid sizeof() uses of things that >> shouldn't be size-able -- this one is noticeable only because the test >> is printing different things. Since you're mentioning me directly, >> please remember that I also proposed the right fix for this bug, that >> is to remove printing the size of structures from ffprobe. > > That change will probably go in as well. If that change goes in, I withdraw any objection on this patch. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Change type of spherical stuff to uint32_t
On Tue, Mar 14, 2017 at 12:12 PM, James Almer wrote: > On 3/14/2017 12:37 PM, Vittorio Giovara wrote: >> On Sun, Mar 12, 2017 at 11:21 PM, Michael Niedermayer >> wrote: >>> On Sun, Mar 12, 2017 at 08:54:07PM -0400, Ronald S. Bultje wrote: >>>> Hi, >>>> >>>> On Sun, Mar 12, 2017 at 7:32 PM, James Almer wrote: >>>> >>>>> On 3/12/2017 6:16 PM, Michael Niedermayer wrote: >>>>>> Using the same type across platforms is more robust and avoids platform >>>>>> specific issues from differences in range. >> >> I still think you are curing the symptom rather than the illness. >> >> Besides, you can't just change types on a whim, you should wait for >> the major bump (if at all). > > As mentioned by Hendrik, it's only six days old and not part of any > release, so it can be changed just fine. Not really, but I'm not here to discuss policies. >>>>>> Also fixed point integers are on a semantical level not size_t >> >> This is only theoretical, > > You specifically wrote the API to have the fields store 0.32 fixed point > values. Why you choose size_t for a field that's meant to store exactly > 32 bits is the question. I choose size_t because it represented a `size` as the name implies, and since it is very closely related to "cropping rectangle" I picked the types of the fields that were in use in AVFrame: https://git.libav.org/?p=libav.git;a=blob;f=libavutil/frame.h;h=f9ffb5bbbf852de4728442a3940c6f2ddb752ecd;hb=HEAD#l385 > Afaik, size_t could even be 16 bit in some systems. FreeDOS is probably > one, and we supposedly support it. Libav does too, so maybe this change > should be done in both projects. I'm pretty sure both projects will fail to build on 16 bits platform. Unless you find a very very smart compiler that will ignore all large constants not fitting into int, all out of bounds shifts, tables not fitting into even 1MB and such. Then it will probably crash somewhere anyway. Someone mentioned me that this would be similar to a certain MPEG reference decoder from Fraunhofer that does not compile right in 64-bit mode and does not run by default because some of its functions require >10MB of stack. >> and, since we're talking about semantics, >> you're breaking ABI by using sizeof(AVSphericalMapping) outside of >> libavutil. > > Well, you're the one that introduced the only current sizeof() check > outside of libavutil, in both projects. Are you sure? There are several invalid sizeof() uses of things that shouldn't be size-able -- this one is noticeable only because the test is printing different things. Since you're mentioning me directly, please remember that I also proposed the right fix for this bug, that is to remove printing the size of structures from ffprobe. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Change type of spherical stuff to uint32_t
On Sun, Mar 12, 2017 at 11:21 PM, Michael Niedermayer wrote: > On Sun, Mar 12, 2017 at 08:54:07PM -0400, Ronald S. Bultje wrote: >> Hi, >> >> On Sun, Mar 12, 2017 at 7:32 PM, James Almer wrote: >> >> > On 3/12/2017 6:16 PM, Michael Niedermayer wrote: >> > > Using the same type across platforms is more robust and avoids platform >> > > specific issues from differences in range. I still think you are curing the symptom rather than the illness. Besides, you can't just change types on a whim, you should wait for the major bump (if at all). >> > > Also fixed point integers are on a semantical level not size_t This is only theoretical, and, since we're talking about semantics, you're breaking ABI by using sizeof(AVSphericalMapping) outside of libavutil. >> > > Previous version reviewed-by: James Almer >> > > Signed-off-by: Michael Niedermayer >> > > --- >> > > ffprobe.c | 2 +- >> > > libavformat/dump.c| 6 +++--- >> > > libavutil/spherical.c | 6 +++--- >> > > libavutil/spherical.h | 16 >> > > 4 files changed, 15 insertions(+), 15 deletions(-) >> > >> > Needs FATE update. >> >> >> Since this is Vittorio's code, it's probably good to ask him [CC]? From > > yes, > > semi on topic: but i think people interrested in FFmpeg > development and the direction the code goes should subscribe to > ffmpeg-devel. This CC-ing is not too practical My subscription to ffmpeg-security was rejected, if you can rectify that, I'll subscribe to ffmpeg-devel too. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] fate: Do not report side data size
On Wed, Mar 8, 2017 at 8:15 PM, James Almer wrote: > On 3/8/2017 9:45 PM, Michael Niedermayer wrote: >> On Wed, Mar 08, 2017 at 11:54:59PM +0100, Hendrik Leppkes wrote: >>> On Wed, Mar 8, 2017 at 3:42 PM, Ronald S. Bultje wrote: Hi, On Wed, Mar 8, 2017 at 9:31 AM, wm4 wrote: > On Wed, 8 Mar 2017 14:09:53 +0100 > Michael Niedermayer wrote: > > If the size printing is removed then other code should be added >> to test for the size to match the correct value > > Then it would be more reasonable to make av_packet_add_side_data() > check whether the size is correct for the given side data type. I think you're both right here, this is a pretty good idea (for fixed-size side-data types). >>> >>> So how do we fix fate now? Change the datatypes to uint32_t, remove >>> the size print out? >> >> why is the data type size_t and not uint32_t int64_t or unsigned int ? >> >> independant of the fate issue i mean, size_t seems a strange choice Well as the name implies, size_t should be usable to represent sizes ;) Beside that, the choice of making these elements size_t was made after the rectangle defined in avframe, whose fields are defined with size_t too. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/matroskaenc: add support for Spherical Video elements
On Wed, Mar 8, 2017 at 4:05 PM, James Almer wrote:
> On 3/8/2017 5:08 PM, Vittorio Giovara wrote:
>> On Wed, Mar 8, 2017 at 2:46 PM, James Almer wrote:
>>> Signed-off-by: James Almer
>>> ---
>>> libavformat/matroskaenc.c | 69
>>> +++
>>> 1 file changed, 69 insertions(+)
>>>
>>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>>> index 1605f0cafe..0ee927d63e 100644
>>> --- a/libavformat/matroskaenc.c
>>> +++ b/libavformat/matroskaenc.c
>>> @@ -918,6 +918,72 @@ static int mkv_write_video_color(AVIOContext *pb,
>>> AVCodecParameters *par, AVStre
>>> return 0;
>>> }
>>>
>>> +static int mkv_write_video_projection(AVIOContext *pb, AVStream *st) {
>>> +int side_data_size = 0;
>>> +const AVSphericalMapping *spherical =
>>> +(const AVSphericalMapping*) av_stream_get_side_data(st,
>>> AV_PKT_DATA_SPHERICAL,
>>> +
>>> &side_data_size);
>>> +
>>> +if (side_data_size == sizeof(AVSphericalMapping)) {
>>
>> I don't think you have to check for this (and checking sizeof() of
>> this struct outside lavu breaks ABI), it's enough to check that size
>> != 0
>
> Ok.
>
> You'll probably also have to do the same on libavformat/dump.c, for
> that matter.
This is valid for most side data there, somebody should do a thorough cleanup
>>> +put_ebml_binary(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPRIVATE,
>>> private, sizeof(private));
>>> +break;
>>> +}
>>> +case AV_SPHERICAL_EQUIRECTANGULAR:
>>> +put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE,
>>> + MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR);
>>> +break;
>>> +case AV_SPHERICAL_CUBEMAP:
>>> +{
>>> +uint8_t private[12];
>>> +put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE,
>>> + MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP);
>>> +AV_WB32(private + 0, 0); // version + flags
>>> +AV_WB32(private + 4, 0); // layout
>>> +AV_WB32(private + 8, spherical->padding);
>>> +put_ebml_binary(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPRIVATE,
>>> private, sizeof(private));
>>> +break;
>>> +}
>>> +default:
>>> +// TODO: Mesh projection once implemented in AVSphericalMapping
>>
>> a little av_log message to warn about this?
>
> This is a muxer, spherical->projection will be one of the supported enums.
> This default case is just to prevent potential compiler warnings and can
> be removed.
>
> What needs a log message is the demuxer.
This is a muxer, that can be created by an API user that does not know
how to read documentation and who initialises spherical->projection
wrong. I didn't say to error out, but I think that adding just a
warning is easier to deal with than having to debug it manually.
>>
>>> +goto end;
>>> +}
>>> +
>>> +put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEYAW,
>>> (double)spherical->yaw / (1 << 16));
>>> +put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEPITCH,
>>> (double)spherical->pitch / (1 << 16));
>>> +put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEROLL,
>>> (double)spherical->roll / (1 << 16));
>>
>> does the matroska spec require plain integers?
>> spherical->{yaw,pitch,roll} are in 16.16 so there should be no need of
>> extra conversions
>
> Matroska stores these three as double precision fp values.
nice
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/matroskaenc: add support for Spherical Video elements
On Wed, Mar 8, 2017 at 2:46 PM, James Almer wrote:
> Signed-off-by: James Almer
> ---
> libavformat/matroskaenc.c | 69
> +++
> 1 file changed, 69 insertions(+)
>
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 1605f0cafe..0ee927d63e 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -918,6 +918,72 @@ static int mkv_write_video_color(AVIOContext *pb,
> AVCodecParameters *par, AVStre
> return 0;
> }
>
> +static int mkv_write_video_projection(AVIOContext *pb, AVStream *st) {
> +int side_data_size = 0;
> +const AVSphericalMapping *spherical =
> +(const AVSphericalMapping*) av_stream_get_side_data(st,
> AV_PKT_DATA_SPHERICAL,
> +&side_data_size);
> +
> +if (side_data_size == sizeof(AVSphericalMapping)) {
I don't think you have to check for this (and checking sizeof() of
this struct outside lavu breaks ABI), it's enough to check that size
!= 0
> +AVIOContext *dyn_cp;
> +uint8_t *projection_ptr;
> +int ret, projection_size;
> +
> +ret = avio_open_dyn_buf(&dyn_cp);
> +if (ret < 0)
> +return ret;
> +
> +switch (spherical->projection) {
> +case AV_SPHERICAL_EQUIRECTANGULAR_TILE:
> +{
> +uint8_t private[20];
> +put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE,
> + MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR);
> +AV_WB32(private + 0, 0); // version + flags
> +AV_WB32(private + 4, spherical->bound_top);
> +AV_WB32(private + 8, spherical->bound_bottom);
> +AV_WB32(private + 12, spherical->bound_left);
> +AV_WB32(private + 16, spherical->bound_right);
I'd feel better if you could use bytestream functions
> +put_ebml_binary(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPRIVATE,
> private, sizeof(private));
> +break;
> +}
> +case AV_SPHERICAL_EQUIRECTANGULAR:
> +put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE,
> + MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR);
> +break;
> +case AV_SPHERICAL_CUBEMAP:
> +{
> +uint8_t private[12];
> +put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE,
> + MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP);
> +AV_WB32(private + 0, 0); // version + flags
> +AV_WB32(private + 4, 0); // layout
> +AV_WB32(private + 8, spherical->padding);
> +put_ebml_binary(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPRIVATE,
> private, sizeof(private));
> +break;
> +}
> +default:
> +// TODO: Mesh projection once implemented in AVSphericalMapping
a little av_log message to warn about this?
> +goto end;
> +}
> +
> +put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEYAW,
> (double)spherical->yaw / (1 << 16));
> +put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEPITCH,
> (double)spherical->pitch / (1 << 16));
> +put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEROLL,
> (double)spherical->roll / (1 << 16));
does the matroska spec require plain integers?
spherical->{yaw,pitch,roll} are in 16.16 so there should be no need of
extra conversions
> +
> +end:
> +projection_size = avio_close_dyn_buf(dyn_cp, &projection_ptr);
> +if (projection_size) {
> +ebml_master projection = start_ebml_master(pb,
> MATROSKA_ID_VIDEOPROJECTION, projection_size);
> +avio_write(pb, projection_ptr, projection_size);
> +end_ebml_master(pb, projection);
> +}
> +av_freep(&projection_ptr);
> +}
> +
> +return 0;
> +}
> +
> static void mkv_write_field_order(AVIOContext *pb, int mode,
>enum AVFieldOrder field_order)
> {
> @@ -1268,6 +1334,9 @@ static int mkv_write_track(AVFormatContext *s,
> MatroskaMuxContext *mkv,
> ret = mkv_write_video_color(pb, par, st);
> if (ret < 0)
> return ret;
> +ret = mkv_write_video_projection(pb, st);
> +if (ret < 0)
> +return ret;
> end_ebml_master(pb, subinfo);
> break;
>
> --
> 2.12.0
>
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] fate: Do not report side data size
On Wed, Mar 8, 2017 at 6:51 AM, Michael Niedermayer wrote: >> Removing the side_data_size from output should be fine, as its a >> implementation detail and as seen here can even vary between >> architecture or possibly even compiler. >> Maybe someone that uses that ffprobe output more often can comment? > > I use all kinds of stuff > if something is removed from ffprobes output it wont be tested anymore. > We should test more not less. We should test better, not more. >> An alternative for fixing fate would be to use fixed size fields in >> the new sidedata, although the possibility of it breaking similarly >> again in the future then remains. > > I strongly prefer fixed size to be used in side data over platform > dependant fields. Not only does size become testable but theres also > a platform specific difference less in the interface which should > help bug reproducability between platforms I won't comment on this statement which I find wrong in many ways, but I'm just going to say that for the case at hand sizeof(AVSphericalMapping) is not part of the ABI, and so it is allowed to have different sizes on different architectures. Same for many other side data objects, this one just happens to use size_t, which is the right type for expressing a size. Printing the size of a struct adds absolutely no value to a test, and changing a type because of a test is a hacky workaround I have no intention to pursue. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] fate: Do not report side data size
This should address the mismatch between different archs Please CC. -- Vittorio 0001-fate-Do-not-report-side-data-size.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] mov: Fix checking layout and loading padding for cubemaps
---
I missed this chunk of review in this version of the set,
sorry, here is the proper patch.
Vittorio
libavformat/mov.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index cc098cd977..d5c3949050 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4635,7 +4635,7 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
{
AVStream *st;
MOVStreamContext *sc;
-int size;
+int size, layout;
int32_t yaw, pitch, roll;
size_t l = 0, t = 0, r = 0, b = 0;
size_t padding = 0;
@@ -4699,6 +4699,12 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
avio_skip(pb, 4); /* version + flags */
switch (tag) {
case MKTAG('c','b','m','p'):
+layout = avio_rb32(pb);
+if (layout) {
+av_log(c->fc, AV_LOG_WARNING,
+ "Unsupported cubemap layout %d\n", layout);
+return 0;
+}
projection = AV_SPHERICAL_CUBEMAP;
padding = avio_rb32(pb);
break;
--
2.12.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] matroskadec: cosmetics: Rearrange checks for projection-depedendent properties
---
... and this chunk for matroska too.
Vittorio
libavformat/matroskadec.c | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index fdc3f268aa..fdb23ab05e 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1930,9 +1930,7 @@ static int mkv_parse_video_projection(AVStream *st, const
MatroskaTrack *track)
switch (track->video.projection.type) {
case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
-if (track->video.projection.private.size == 0)
-projection = AV_SPHERICAL_EQUIRECTANGULAR;
-else if (track->video.projection.private.size == 20) {
+if (track->video.projection.private.size == 20) {
t = bytestream2_get_be32(&gb);
b = bytestream2_get_be32(&gb);
l = bytestream2_get_be32(&gb);
@@ -1946,15 +1944,15 @@ static int mkv_parse_video_projection(AVStream *st,
const MatroskaTrack *track)
l, t, r, b);
return AVERROR_INVALIDDATA;
}
-
-if (l || t || r || b)
-projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
-else
-projection = AV_SPHERICAL_EQUIRECTANGULAR;
-} else {
+} else if (track->video.projection.private.size != 0) {
av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
return AVERROR_INVALIDDATA;
}
+
+if (l || t || r || b)
+projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
+else
+projection = AV_SPHERICAL_EQUIRECTANGULAR;
break;
case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP:
if (track->video.projection.private.size < 4) {
@@ -1962,13 +1960,12 @@ static int mkv_parse_video_projection(AVStream *st,
const MatroskaTrack *track)
return AVERROR_INVALIDDATA;
} else if (track->video.projection.private.size == 12) {
uint32_t layout = bytestream2_get_be32(&gb);
-if (layout == 0) {
-projection = AV_SPHERICAL_CUBEMAP;
-} else {
+if (layout) {
av_log(NULL, AV_LOG_WARNING,
"Unknown spherical cubemap layout %"PRIu32"\n", layout);
return 0;
}
+projection = AV_SPHERICAL_CUBEMAP;
padding = bytestream2_get_be32(&gb);
} else {
av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
--
2.12.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv3 3/3] mkv: Export bounds and padding from spherical metadata
On Tue, Feb 28, 2017 at 1:15 PM, James Almer wrote:
> On 2/28/2017 2:46 PM, Vittorio Giovara wrote:
>> On Tue, Feb 28, 2017 at 11:00 AM, James Almer wrote:
>>> On 2/22/2017 1:21 PM, James Almer wrote:
>>>> On 2/21/2017 7:35 PM, Vittorio Giovara wrote:
>>>
>>> CCing this one as well.
>>>
>>>>> ---
>>>>> libavformat/matroskadec.c | 64
>>>>> --
>>>>> tests/ref/fate/matroska-spherical-mono | 6 +++-
>>>>> 2 files changed, 66 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>>>>> index 7223e94..0a02237 100644
>>>>> --- a/libavformat/matroskadec.c
>>>>> +++ b/libavformat/matroskadec.c
>>>>> @@ -1913,16 +1913,65 @@ static int mkv_parse_video_projection(AVStream
>>>>> *st, const MatroskaTrack *track)
>>>>> AVSphericalMapping *spherical;
>>>>> enum AVSphericalProjection projection;
>>>>> size_t spherical_size;
>>>>> +size_t l, t, r, b;
>>>>> +size_t padding = 0;
>>>>> int ret;
>>>>> +GetByteContext gb;
>>>>> +
>>>>> +bytestream2_init(&gb, track->video.projection.private.data,
>>>>> + track->video.projection.private.size);
>>>>
>>>> private.size can be zero and private.data NULL. bytestream2_init()
>>>> will trigger an assert in those cases.
>>
>> :( asserts like this are really dampening, it should be on read not on
>> creation (if at all)
>
> True, guess it should return an error value instead.
For now what should we do? Just check that private.size and
private.data are non-0 and return with an error otherwise?
>>>>> +if (bytestream2_get_byte(&gb) != 0) {
>>>>> +av_log(NULL, AV_LOG_WARNING, "Unknown spherical metadata\n");
>>>>> +return 0;
>>>>> +}
>>>>> +
>>>>> +bytestream2_skip(&gb, 3); // flags
>>>>>
>>>>> switch (track->video.projection.type) {
>>>>> case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
>>>>> -projection = AV_SPHERICAL_EQUIRECTANGULAR;
>>>>> +if (track->video.projection.private.size == 0)
>>>>> +projection = AV_SPHERICAL_EQUIRECTANGULAR;
>>>>> +else if (track->video.projection.private.size == 20) {
>>>>> +t = bytestream2_get_be32(&gb);
>>>>> +b = bytestream2_get_be32(&gb);
>>>>> +l = bytestream2_get_be32(&gb);
>>>>> +r = bytestream2_get_be32(&gb);
>>>>> +
>>>>> +if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
>>>>> +av_log(NULL, AV_LOG_ERROR,
>>>>> + "Invalid bounding rectangle coordinates "
>>>>> + "%zu,%zu,%zu,%zu\n", l, t, r, b);
>>>>
>>>> Use SIZE_SPECIFIER instead of zu. Msvc doesn't like the latter.
>>>> Same with the mov implementation.
>>
>> Umh? Isn't %zu supported on any msvc version that matters (2015 onwards)?
>> https://connect.microsoft.com/VisualStudio/feedback/details/2083820/printf-format-specifier-zu-is-supported-but-not-documented
>
> We also support 2012 and 2013. SIZE_SPECIFIER becomes zu for gcc and
> such, but Iu for MSVC.
old software..
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv3 3/3] mkv: Export bounds and padding from spherical metadata
On Tue, Feb 28, 2017 at 12:46 PM, Vittorio Giovara
wrote:
>>
>> I think this'll look better as
>>
>>
>> case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
>> projection = AV_SPHERICAL_EQUIRECTANGULAR;
>>
>> if (track->video.projection.private.size == 20) {
>> [...]
>> if (l || t || r || b)
>> projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
>> } else if (track->video.projection.private.size != 0) {
>> // return error
>> }
>
> Sorry, I don't follow, what is your suggestion?
nevermind, i get it, and ok
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv3 3/3] mkv: Export bounds and padding from spherical metadata
On Tue, Feb 28, 2017 at 11:00 AM, James Almer wrote:
> On 2/22/2017 1:21 PM, James Almer wrote:
>> On 2/21/2017 7:35 PM, Vittorio Giovara wrote:
>
> CCing this one as well.
>
>>> ---
>>> libavformat/matroskadec.c | 64
>>> --
>>> tests/ref/fate/matroska-spherical-mono | 6 +++-
>>> 2 files changed, 66 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>>> index 7223e94..0a02237 100644
>>> --- a/libavformat/matroskadec.c
>>> +++ b/libavformat/matroskadec.c
>>> @@ -1913,16 +1913,65 @@ static int mkv_parse_video_projection(AVStream *st,
>>> const MatroskaTrack *track)
>>> AVSphericalMapping *spherical;
>>> enum AVSphericalProjection projection;
>>> size_t spherical_size;
>>> +size_t l, t, r, b;
>>> +size_t padding = 0;
>>> int ret;
>>> +GetByteContext gb;
>>> +
>>> +bytestream2_init(&gb, track->video.projection.private.data,
>>> + track->video.projection.private.size);
>>
>> private.size can be zero and private.data NULL. bytestream2_init()
>> will trigger an assert in those cases.
:( asserts like this are really dampening, it should be on read not on
creation (if at all)
>>> +
>>> +if (bytestream2_get_byte(&gb) != 0) {
>>> +av_log(NULL, AV_LOG_WARNING, "Unknown spherical metadata\n");
>>> +return 0;
>>> +}
>>> +
>>> +bytestream2_skip(&gb, 3); // flags
>>>
>>> switch (track->video.projection.type) {
>>> case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
>>> -projection = AV_SPHERICAL_EQUIRECTANGULAR;
>>> +if (track->video.projection.private.size == 0)
>>> +projection = AV_SPHERICAL_EQUIRECTANGULAR;
>>> +else if (track->video.projection.private.size == 20) {
>>> +t = bytestream2_get_be32(&gb);
>>> +b = bytestream2_get_be32(&gb);
>>> +l = bytestream2_get_be32(&gb);
>>> +r = bytestream2_get_be32(&gb);
>>> +
>>> +if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
>>> +av_log(NULL, AV_LOG_ERROR,
>>> + "Invalid bounding rectangle coordinates "
>>> + "%zu,%zu,%zu,%zu\n", l, t, r, b);
>>
>> Use SIZE_SPECIFIER instead of zu. Msvc doesn't like the latter.
>> Same with the mov implementation.
Umh? Isn't %zu supported on any msvc version that matters (2015 onwards)?
https://connect.microsoft.com/VisualStudio/feedback/details/2083820/printf-format-specifier-zu-is-supported-but-not-documented
>>> +return AVERROR_INVALIDDATA;
>>> +}
>>> +
>>> +if (l || t || r || b)
>>> +projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
>>> +else
>>> +projection = AV_SPHERICAL_EQUIRECTANGULAR;
>>> +} else {
>>> +av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
>>> +return AVERROR_INVALIDDATA;
>>> +}
>>> break;
>>
>> Nit: I still think the Equi case looks better the way i suggested in
>> my previous email.
>
> And what i wrote in said previous email that i also didn't CC:
>
>
> I think this'll look better as
>
>
> case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
> projection = AV_SPHERICAL_EQUIRECTANGULAR;
>
> if (track->video.projection.private.size == 20) {
> [...]
> if (l || t || r || b)
> projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
> } else if (track->video.projection.private.size != 0) {
> // return error
> }
Sorry, I don't follow, what is your suggestion?
> ---
>
>>
>>> case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP:
>>> -if (track->video.projection.private.size < 4)
>>> +if (track->video.projection.private.size < 4) {
>>> +av_log(NULL, AV_LOG_ERROR, "Missing projection private
>>> properties\n");
>>> +return AVERROR_INVALIDDATA;
>>> +} else if (track->video.projection.private.size == 12) {
>>> +uint32_t layout = bytestream2_get_be32(
Re: [FFmpeg-devel] [PATCHv3 2/3] mov: Export bounds and padding from spherical metadata
On Tue, Feb 28, 2017 at 10:58 AM, James Almer wrote:
> On 2/21/2017 8:07 PM, James Almer wrote:
>> On 2/21/2017 7:35 PM, Vittorio Giovara wrote:
>>> Update the fate test as needed.
>>> ---
>>> libavformat/mov.c | 28 +++-
>>> tests/ref/fate/mov-spherical-mono | 6 +-
>>> 2 files changed, 32 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>>> index 7b0bbcc..d798336 100644
>>> --- a/libavformat/mov.c
>>> +++ b/libavformat/mov.c
>>> @@ -4637,6 +4637,8 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext
>>> *pb, MOVAtom atom)
>>> MOVStreamContext *sc;
>>> int size, version;
>>> int32_t yaw, pitch, roll;
>>> +size_t l, t, r, b;
>>> +size_t padding = 0;
>>> uint32_t tag;
>>> enum AVSphericalProjection projection;
>>>
>>> @@ -4716,9 +4718,25 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext
>>> *pb, MOVAtom atom)
>>> switch (tag) {
>>> case MKTAG('c','b','m','p'):
>>> projection = AV_SPHERICAL_CUBEMAP;
>>> +padding = avio_rb32(pb);
>>
>> Doesn't layout come first?
Ah, that's right, thanks
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCHv3 1/3] spherical: Add tiled equirectangular type and projection-specific properties
Signed-off-by: Vittorio Giovara
---
This leaves bounds unchanged, simplifying future muxing code.
Add a convenience function where human-readable values are needed.
Update mov and mkv in subsequent patches.
Vittorio
doc/APIchanges | 5 +++
ffprobe.c | 19 +++--
libavformat/dump.c | 15 ++-
libavutil/spherical.c | 18 +
libavutil/spherical.h | 74 ++
libavutil/version.h| 2 +-
tests/ref/fate/matroska-spherical-mono | 2 +-
tests/ref/fate/mov-spherical-mono | 2 +-
8 files changed, 128 insertions(+), 9 deletions(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index d739895..569d457 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,11 @@ libavutil: 2015-08-28
API changes, most recent first:
+2017-02-10 - xxx - lavu 55.48.100 / 55.33.0 - spherical.h
+ Add AV_SPHERICAL_EQUIRECTANGULAR_TILE, av_spherical_tile_bounds(),
+ and projection-specific properties (bound_left, bound_top, bound_right,
+ bound_bottom, padding) to AVSphericalMapping.
+
2017-02-13 - xxx - lavc 57.80.100 - avcodec.h
Add AVCodecContext.hw_device_ctx.
diff --git a/ffprobe.c b/ffprobe.c
index 046f080..c85c3a1 100644
--- a/ffprobe.c
+++ b/ffprobe.c
@@ -1762,6 +1762,7 @@ static inline int show_tags(WriterContext *w,
AVDictionary *tags, int section_id
}
static void print_pkt_side_data(WriterContext *w,
+AVCodecParameters *par,
const AVPacketSideData *side_data,
int nb_side_data,
SectionID id_data_list,
@@ -1788,9 +1789,19 @@ static void print_pkt_side_data(WriterContext *w,
const AVSphericalMapping *spherical = (AVSphericalMapping
*)sd->data;
if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR)
print_str("projection", "equirectangular");
-else if (spherical->projection == AV_SPHERICAL_CUBEMAP)
+else if (spherical->projection == AV_SPHERICAL_CUBEMAP) {
print_str("projection", "cubemap");
-else
+print_int("padding", spherical->padding);
+} else if (spherical->projection ==
AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
+size_t l, t, r, b;
+av_spherical_tile_bounds(spherical, par->width, par->height,
+ &l, &t, &r, &b);
+print_str("projection", "tiled equirectangular");
+print_int("bound_left", l);
+print_int("bound_top", t);
+print_int("bound_right", r);
+print_int("bound_bottom", b);
+} else
print_str("projection", "unknown");
print_int("yaw", (double) spherical->yaw / (1 << 16));
@@ -1843,7 +1854,7 @@ static void show_packet(WriterContext *w, InputFile
*ifile, AVPacket *pkt, int p
av_dict_free(&dict);
}
-print_pkt_side_data(w, pkt->side_data, pkt->side_data_elems,
+print_pkt_side_data(w, st->codecpar, pkt->side_data,
pkt->side_data_elems,
SECTION_ID_PACKET_SIDE_DATA_LIST,
SECTION_ID_PACKET_SIDE_DATA);
}
@@ -2404,7 +2415,7 @@ static int show_stream(WriterContext *w, AVFormatContext
*fmt_ctx, int stream_id
ret = show_tags(w, stream->metadata, in_program ?
SECTION_ID_PROGRAM_STREAM_TAGS : SECTION_ID_STREAM_TAGS);
if (stream->nb_side_data) {
-print_pkt_side_data(w, stream->side_data, stream->nb_side_data,
+print_pkt_side_data(w, stream->codecpar, stream->side_data,
stream->nb_side_data,
SECTION_ID_STREAM_SIDE_DATA_LIST,
SECTION_ID_STREAM_SIDE_DATA);
}
diff --git a/libavformat/dump.c b/libavformat/dump.c
index d9aa3af..505d572 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -343,7 +343,7 @@ static void dump_mastering_display_metadata(void *ctx,
AVPacketSideData* sd) {
av_q2d(metadata->min_luminance), av_q2d(metadata->max_luminance));
}
-static void dump_spherical(void *ctx, AVPacketSideData *sd)
+static void dump_spherical(void *ctx, AVCodecParameters *par, AVPacketSideData
*sd)
{
AVSphericalMapping *spherical = (AVSphericalMapping *)sd->data;
double yaw, pitch, roll;
@@ -357,6 +357,8 @@ static void dump_spherical(void *ctx, AVPacketSideData *sd)
av_log(ctx, AV_LOG_INFO, "equirectangular ");
else if (spherical->projection == AV_SPHERICAL_CUBEMAP)
av_log(ctx, AV_LOG_INFO, "c
[FFmpeg-devel] [PATCHv3 3/3] mkv: Export bounds and padding from spherical metadata
---
libavformat/matroskadec.c | 64 --
tests/ref/fate/matroska-spherical-mono | 6 +++-
2 files changed, 66 insertions(+), 4 deletions(-)
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 7223e94..0a02237 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1913,16 +1913,65 @@ static int mkv_parse_video_projection(AVStream *st,
const MatroskaTrack *track)
AVSphericalMapping *spherical;
enum AVSphericalProjection projection;
size_t spherical_size;
+size_t l, t, r, b;
+size_t padding = 0;
int ret;
+GetByteContext gb;
+
+bytestream2_init(&gb, track->video.projection.private.data,
+ track->video.projection.private.size);
+
+if (bytestream2_get_byte(&gb) != 0) {
+av_log(NULL, AV_LOG_WARNING, "Unknown spherical metadata\n");
+return 0;
+}
+
+bytestream2_skip(&gb, 3); // flags
switch (track->video.projection.type) {
case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
-projection = AV_SPHERICAL_EQUIRECTANGULAR;
+if (track->video.projection.private.size == 0)
+projection = AV_SPHERICAL_EQUIRECTANGULAR;
+else if (track->video.projection.private.size == 20) {
+t = bytestream2_get_be32(&gb);
+b = bytestream2_get_be32(&gb);
+l = bytestream2_get_be32(&gb);
+r = bytestream2_get_be32(&gb);
+
+if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
+av_log(NULL, AV_LOG_ERROR,
+ "Invalid bounding rectangle coordinates "
+ "%zu,%zu,%zu,%zu\n", l, t, r, b);
+return AVERROR_INVALIDDATA;
+}
+
+if (l || t || r || b)
+projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
+else
+projection = AV_SPHERICAL_EQUIRECTANGULAR;
+} else {
+av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
+return AVERROR_INVALIDDATA;
+}
break;
case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP:
-if (track->video.projection.private.size < 4)
+if (track->video.projection.private.size < 4) {
+av_log(NULL, AV_LOG_ERROR, "Missing projection private
properties\n");
+return AVERROR_INVALIDDATA;
+} else if (track->video.projection.private.size == 12) {
+uint32_t layout = bytestream2_get_be32(&gb);
+if (layout == 0) {
+projection = AV_SPHERICAL_CUBEMAP;
+} else {
+av_log(NULL, AV_LOG_WARNING,
+ "Unknown spherical cubemap layout %"PRIu32"\n", layout);
+return 0;
+}
+padding = bytestream2_get_be32(&gb);
+} else {
+av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
return AVERROR_INVALIDDATA;
-projection = AV_SPHERICAL_CUBEMAP;
+}
break;
default:
return 0;
@@ -1937,6 +1986,15 @@ static int mkv_parse_video_projection(AVStream *st,
const MatroskaTrack *track)
spherical->pitch = (int32_t)(track->video.projection.pitch * (1 << 16));
spherical->roll = (int32_t)(track->video.projection.roll * (1 << 16));
+spherical->padding = padding;
+
+if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
+spherical->bound_left = l;
+spherical->bound_top= t;
+spherical->bound_right = r;
+spherical->bound_bottom = b;
+}
+
ret = av_stream_add_side_data(st, AV_PKT_DATA_SPHERICAL, (uint8_t
*)spherical,
spherical_size);
if (ret < 0) {
diff --git a/tests/ref/fate/matroska-spherical-mono
b/tests/ref/fate/matroska-spherical-mono
index 8048aff..a70d879 100644
--- a/tests/ref/fate/matroska-spherical-mono
+++ b/tests/ref/fate/matroska-spherical-mono
@@ -8,7 +8,11 @@ inverted=0
[SIDE_DATA]
side_data_type=Spherical Mapping
side_data_size=56
-projection=equirectangular
+projection=tiled equirectangular
+bound_left=148
+bound_top=73
+bound_right=147
+bound_bottom=72
yaw=45
pitch=30
roll=15
--
2.10.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCHv3 2/3] mov: Export bounds and padding from spherical metadata
Update the fate test as needed.
---
libavformat/mov.c | 28 +++-
tests/ref/fate/mov-spherical-mono | 6 +-
2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 7b0bbcc..d798336 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4637,6 +4637,8 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
MOVStreamContext *sc;
int size, version;
int32_t yaw, pitch, roll;
+size_t l, t, r, b;
+size_t padding = 0;
uint32_t tag;
enum AVSphericalProjection projection;
@@ -4716,9 +4718,25 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
switch (tag) {
case MKTAG('c','b','m','p'):
projection = AV_SPHERICAL_CUBEMAP;
+padding = avio_rb32(pb);
break;
case MKTAG('e','q','u','i'):
-projection = AV_SPHERICAL_EQUIRECTANGULAR;
+t = avio_rb32(pb);
+b = avio_rb32(pb);
+l = avio_rb32(pb);
+r = avio_rb32(pb);
+
+if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
+av_log(c->fc, AV_LOG_ERROR,
+ "Invalid bounding rectangle coordinates "
+ "%zu,%zu,%zu,%zu\n", l, t, r, b);
+return AVERROR_INVALIDDATA;
+}
+
+if (l || t || r || b)
+projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
+else
+projection = AV_SPHERICAL_EQUIRECTANGULAR;
break;
default:
av_log(c->fc, AV_LOG_ERROR, "Unknown projection type\n");
@@ -4735,6 +4753,14 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
sc->spherical->pitch = pitch;
sc->spherical->roll = roll;
+sc->spherical->padding = padding;
+
+if (projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
+sc->spherical->bound_left = l;
+sc->spherical->bound_top= t;
+sc->spherical->bound_right = r;
+sc->spherical->bound_bottom = b;
+}
return 0;
}
diff --git a/tests/ref/fate/mov-spherical-mono
b/tests/ref/fate/mov-spherical-mono
index 8048aff..a70d879 100644
--- a/tests/ref/fate/mov-spherical-mono
+++ b/tests/ref/fate/mov-spherical-mono
@@ -8,7 +8,11 @@ inverted=0
[SIDE_DATA]
side_data_type=Spherical Mapping
side_data_size=56
-projection=equirectangular
+projection=tiled equirectangular
+bound_left=148
+bound_top=73
+bound_right=147
+bound_bottom=72
yaw=45
pitch=30
roll=15
--
2.10.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] spherical: Add tiled equirectangular type and projection-specific properties
On Thu, Feb 16, 2017 at 3:41 PM, Aaron Colwell wrote: > After sleeping on this, I think what you have will be fine. Resizing a > cubemap w/ padding is just going to have to be handled in a special way > because fractional pixel padding isn't going to work right on the GPU. You > really only want to waste a few pixels on padding, not a constant fraction > of the whole frame. Given that we have to handle cubemaps in a special way > for resizing, then my thoughts about resizing equirect aren't really a big > deal. I don't expect any spec changes will be needed for this. Hey Aaron, thanks for the feedback, I actually had some time to think about this (yay holidays), and I'm starting to agree with you: leaving the values for the bounds unchanged is probably better. I think I'll just add a convenience function to simplify conversion, and I'll send an updated set soon. Cheers -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] spherical: Add tiled equirectangular type and projection-specific properties
On Wed, Feb 15, 2017 at 2:54 PM, Aaron Colwell wrote: > Hi Vittorio, > > This may not be a blocker for this patch, but one issue with converting the > bounds to pixels like you do here is that resizing a video could result in > incorrect metadata being generated when muxing. If you keep the bounds in > the 0.0-1.0 fixed point space this problem doesn't happen since it is a > resolution independent representation. If you still want to use pixels here > then the resizing filter will need to become aware of AVSphericalMapping and > adjust it accordingly. I think cubemap padding in pixels may have a similar > issue. This is no way intended to be a blocking comment. It is just meant to > raise awareness on something you might not have considered. Hi Aaron, this is an interesting point, but leaves up open questions. Do you foresee actual cases in which this is going to be a problem? It's not the first time we have fixed point fields exported, but it's also true that the conversion from a 0.32 system is not very common and could leave users puzzled. Regarding padding, is the specification going to be updated to fix this potential issue? Thank you -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] spherical: Add tiled equirectangular type and projection-specific properties
Hi Aaron, On Wed, Feb 15, 2017 at 11:48 AM, Aaron Colwell wrote: >> If the spec changes, it will be the contents of the equi/cbmp/mesh. >> By exporting them raw as extradata, said changes in the spec would >> require no changes to our implementation. > > > This is one of the main reasons I was suggesting this path. I think of these > extradata fields much like the extra data that codecs have. It really is > only important to the code that needs to render a specific projection. For > transcoding, you mainly just need to convey the information in a lossless > manner from demuxer to muxer. I understand that, but "transcoding" is not the only task performed here. Yes, I agree that for your single usecase of converting mp4 metadata to mkv metadata your solution a feasible one, but the code here also has 1. convey information to the users 2. fill in side data and frame data (remember this is a pkt side data that is exported to frames since spherical is a frame property, it doesn't matter that currently the spec is container level only for now) 3. offer it to any muxer for additional parsing You can't expect to fill in binary data and have all three places behave correctly, and handle multiple theoretical version while at it. You need an abstraction like the one provided by the API in this patch. > I anticipate the spec will change in the future. My plan is that no change > will break what is currently specified in the spec right now, but I > anticipate some changes will be made. Having a solution that can gracefully > handle this would be nice. A *binary* solution is certainly not nice, especially if you have multiple version. Besides, you should really make an effort to change the spec as little as possible now that it is public, as more and more software will depend on it and you can't realistically expect that everyone will be always up-to-date (ffmpeg included). >> >> Wouldn't it be a better idea to export the binary data of the >> >> equi/cbmp/mesh boxes into an extradata-like field in the >> >> AVSphericalMapping struct, and let the downstream application parse >> >> it instead? >> > >> > No I don't think so, lavf is an abstraction layer and one of its tasks >> > is to provide a (simple?) unified entry layer. and letting the user >> > parse binary data is IMO bad design and very fragile. Also it's not >> > impossible that another standard for tagging spherical metadata >> > emerges in the future: the current API could very easily wrap it, >> > while exporting the binary entry would be too specification-specific >> > and it would be tied too heavily on the google draft. >> > > I agree with Vittorio that having some form of abstraction is a good thing > and having binary data in places can be problematic. It feels like we could > find some middle ground here by providing helper functions that parse the > binary data into projection specific structs and back just like codecs code > tends to do. I feel like this provides a reasonable balance between having a > common set of fields where things actually have common semantics like > projection_type, yaw/pitch/roll, & extra_data while also providing a way to > get access to projection specific information in a simple way. I don't feel like this has anything to do with the current patchset. This set only allows for properly conveying information in a clean matten to the user (and a muxer if you will can be consider as a plain user). Having a single parsing routine is IMO a useless optimization, and can be discussed in another thread. > At the end of the day players really just need to care about a rendering > mesh so in some sense it would be nice to have that be the abstraction for > the player use case. That is basically what we have done in our internal > player implementations. That could easily be handled by helper functions, > but would be a bad representation for AVSphericalMapping because it would > make transcoding/transmuxing painful. Again, a "player" is not your single user here, you have probing tool that need to provide data understandable by human people. And please stop saying transcoding/transmuxing is painful, on the opposite it's incredibly simple, you just need to read AVSphericalMapping (exactly as any other user) and write down this information. If you think that new (non-binary) fields should be added to further simplify muxing, please do tell. >> Wait for Aaron's opinion before addressing reviews and pushing. He >> sent a different patchset himself and it wouldn't be nice to push >> yours without at least giving him a chance to comment. >> > > Thank you. I really appreciate this. I hope it is clear I'm really trying to > find some middle ground here. My team has been working on this stuff inside > Google for several years and I'm just trying to bring some of that > experience and learning into the open here. I defer to you because this is > your project and I am relatively sparse contributor. Please don't get me wrong, it's rea
[FFmpeg-devel] [PATCHv2 1/4] spherical: Add tiled equirectangular type and projection-specific properties
Signed-off-by: Vittorio Giovara
---
Updated with fate changes and more consistent names.
Please CC.
Vittorio
doc/APIchanges | 5 +++
ffprobe.c | 11 +--
libavformat/dump.c | 10 ++
libavutil/spherical.h | 56 ++
libavutil/version.h| 2 +-
tests/ref/fate/matroska-spherical-mono | 2 +-
tests/ref/fate/mov-spherical-mono | 2 +-
7 files changed, 83 insertions(+), 5 deletions(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index d739895..663bab1 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,11 @@ libavutil: 2015-08-28
API changes, most recent first:
+2017-02-10 - xxx - lavu 55.48.100 / 55.33.0 - spherical.h
+ Add AV_SPHERICAL_EQUIRECTANGULAR_TILE, and projection-specific properties
+ (bound_left, bound_top, bound_right, bound_bottom, padding) to
+ AVSphericalMapping.
+
2017-02-13 - xxx - lavc 57.80.100 - avcodec.h
Add AVCodecContext.hw_device_ctx.
diff --git a/ffprobe.c b/ffprobe.c
index 046f080..acda9bc 100644
--- a/ffprobe.c
+++ b/ffprobe.c
@@ -1788,9 +1788,16 @@ static void print_pkt_side_data(WriterContext *w,
const AVSphericalMapping *spherical = (AVSphericalMapping
*)sd->data;
if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR)
print_str("projection", "equirectangular");
-else if (spherical->projection == AV_SPHERICAL_CUBEMAP)
+else if (spherical->projection == AV_SPHERICAL_CUBEMAP) {
print_str("projection", "cubemap");
-else
+print_int("padding", spherical->padding);
+} else if (spherical->projection ==
AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
+print_str("projection", "tiled equirectangular");
+print_int("bound_left", spherical->bound_left);
+print_int("bound_top", spherical->bound_top);
+print_int("bound_right", spherical->bound_right);
+print_int("bound_bottom", spherical->bound_bottom);
+} else
print_str("projection", "unknown");
print_int("yaw", (double) spherical->yaw / (1 << 16));
diff --git a/libavformat/dump.c b/libavformat/dump.c
index d9aa3af..ae93c7a 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -357,6 +357,8 @@ static void dump_spherical(void *ctx, AVPacketSideData *sd)
av_log(ctx, AV_LOG_INFO, "equirectangular ");
else if (spherical->projection == AV_SPHERICAL_CUBEMAP)
av_log(ctx, AV_LOG_INFO, "cubemap ");
+else if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE)
+av_log(ctx, AV_LOG_INFO, "tiled equirectangular ");
else {
av_log(ctx, AV_LOG_WARNING, "unknown");
return;
@@ -366,6 +368,14 @@ static void dump_spherical(void *ctx, AVPacketSideData *sd)
pitch = ((double)spherical->pitch) / (1 << 16);
roll = ((double)spherical->roll) / (1 << 16);
av_log(ctx, AV_LOG_INFO, "(%f/%f/%f) ", yaw, pitch, roll);
+
+if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
+av_log(ctx, AV_LOG_INFO, "[%zu, %zu, %zu, %zu] ",
+ spherical->bound_left, spherical->bound_top,
+ spherical->bound_right, spherical->bound_bottom);
+} else if (spherical->projection == AV_SPHERICAL_CUBEMAP) {
+av_log(ctx, AV_LOG_INFO, "[pad %zu] ", spherical->padding);
+}
}
static void dump_sidedata(void *ctx, AVStream *st, const char *indent)
diff --git a/libavutil/spherical.h b/libavutil/spherical.h
index eeda625..3c9f3a5 100644
--- a/libavutil/spherical.h
+++ b/libavutil/spherical.h
@@ -63,6 +63,13 @@ enum AVSphericalProjection {
* to the back.
*/
AV_SPHERICAL_CUBEMAP,
+
+/**
+ * Video represents a portion of a sphere mapped on a flat surface
+ * using equirectangular projection. The @ref bounding fields indicate
+ * the position of the current video in a larger surface.
+ */
+AV_SPHERICAL_EQUIRECTANGULAR_TILE,
};
/**
@@ -122,6 +129,55 @@ typedef struct AVSphericalMapping {
/**
* @}
*/
+
+/**
+ * @name Bounding rectangle
+ * @anchor bounding
+ * @{
+ * These fields indicate the location of the current tile, and where
+ * it should be mapped relative to the original surface.
+ *
+ * @code{.unparsed}
+ * ++--+
+ * ||bound_top |
+ * |++ |
+ * | bound_left |tile| |
+ * +<-->||<--->+bound_right
+
[FFmpeg-devel] [PATCHv2 3/4] mkv: Export bounds and padding from spherical metadata
---
Updated according to James' review.
Please CC.
Vittorio
libavformat/matroskadec.c | 69 --
tests/ref/fate/matroska-spherical-mono | 6 ++-
2 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 7223e94..6fa961e 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1913,16 +1913,65 @@ static int mkv_parse_video_projection(AVStream *st,
const MatroskaTrack *track)
AVSphericalMapping *spherical;
enum AVSphericalProjection projection;
size_t spherical_size;
+size_t l, t, r, b;
+size_t padding = 0;
int ret;
+GetByteContext gb;
+
+bytestream2_init(&gb, track->video.projection.private.data,
+ track->video.projection.private.size);
+
+if (bytestream2_get_byte(&gb) != 0) {
+av_log(NULL, AV_LOG_WARNING, "Unknown spherical metadata\n");
+return 0;
+}
+
+bytestream2_skip(&gb, 3); // flags
switch (track->video.projection.type) {
case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
-projection = AV_SPHERICAL_EQUIRECTANGULAR;
+if (track->video.projection.private.size == 0)
+projection = AV_SPHERICAL_EQUIRECTANGULAR;
+else if (track->video.projection.private.size == 20) {
+t = bytestream2_get_be32(&gb);
+b = bytestream2_get_be32(&gb);
+l = bytestream2_get_be32(&gb);
+r = bytestream2_get_be32(&gb);
+
+if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
+av_log(NULL, AV_LOG_ERROR,
+ "Invalid bounding rectangle coordinates "
+ "%zu,%zu,%zu,%zu\n", l, t, r, b);
+return AVERROR_INVALIDDATA;
+}
+
+if (l || t || r || b)
+projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
+else
+projection = AV_SPHERICAL_EQUIRECTANGULAR;
+} else {
+av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
+return AVERROR_INVALIDDATA;
+}
break;
case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP:
-if (track->video.projection.private.size < 4)
+if (track->video.projection.private.size < 4) {
+av_log(NULL, AV_LOG_ERROR, "Missing projection private
properties\n");
+return AVERROR_INVALIDDATA;
+} else if (track->video.projection.private.size == 12) {
+uint32_t layout = bytestream2_get_be32(&gb);
+if (layout == 0) {
+projection = AV_SPHERICAL_CUBEMAP;
+} else {
+av_log(NULL, AV_LOG_WARNING,
+ "Unknown spherical cubemap layout %"PRIu32"\n", layout);
+return 0;
+}
+padding = bytestream2_get_be32(&gb);
+} else {
+av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
return AVERROR_INVALIDDATA;
-projection = AV_SPHERICAL_CUBEMAP;
+}
break;
default:
return 0;
@@ -1937,6 +1986,20 @@ static int mkv_parse_video_projection(AVStream *st,
const MatroskaTrack *track)
spherical->pitch = (int32_t)(track->video.projection.pitch * (1 << 16));
spherical->roll = (int32_t)(track->video.projection.roll * (1 << 16));
+spherical->padding = padding;
+
+if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
+/* conversion from 0.32 coordinates to pixels */
+size_t orig_width = (size_t) track->video.pixel_width * UINT32_MAX /
(UINT32_MAX - r - l);
+size_t orig_height = (size_t) track->video.pixel_height * UINT32_MAX /
(UINT32_MAX - b - t);
+
+/* add a (UINT32_MAX - 1) to round up integer division */
+spherical->bound_left = (orig_width * l + UINT32_MAX - 1) /
UINT32_MAX;
+spherical->bound_top= (orig_height * t + UINT32_MAX - 1) /
UINT32_MAX;
+spherical->bound_right = orig_width - track->video.pixel_width -
spherical->bound_left;
+spherical->bound_bottom = orig_height - track->video.pixel_height -
spherical->bound_top;
+}
+
ret = av_stream_add_side_data(st, AV_PKT_DATA_SPHERICAL, (uint8_t
*)spherical,
spherical_size);
if (ret < 0) {
diff --git a/tests/ref/fate/matroska-spherical-mono
b/tests/ref/fate/matroska-spherical-mono
index 8048aff..a70d879 100644
--- a/tests/ref/fate/matroska-spherical-mono
+++ b/tests/ref/fate/matroska-spherical-mono
@@ -8,7 +8,11 @@ inverted=0
[SIDE_DATA]
side_data_type=Spherical Mapping
side_data_size=56
-projection=equirectangular
+projection=tiled equirectangular
+bound_left=148
+bound_top=73
+bound_right=147
+bound_bottom=72
yaw=45
pitch=30
roll=15
--
2.10.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCHv2 2/4] mov: Export bounds and padding from spherical metadata
Update the fate test as needed.
---
V2 bounds are validated and UINT32_MAX is used.
Please CC.
Vittorio
libavformat/mov.c | 53 ++-
tests/ref/fate/mov-spherical-mono | 6 -
2 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index b518177..a1774b3 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4625,6 +4625,8 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
MOVStreamContext *sc;
int size;
int32_t yaw, pitch, roll;
+size_t l, t, r, b;
+size_t padding = 0;
uint32_t tag;
enum AVSphericalProjection projection;
@@ -4686,9 +4688,25 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
switch (tag) {
case MKTAG('c','b','m','p'):
projection = AV_SPHERICAL_CUBEMAP;
+padding = avio_rb32(pb);
break;
case MKTAG('e','q','u','i'):
-projection = AV_SPHERICAL_EQUIRECTANGULAR;
+t = avio_rb32(pb);
+b = avio_rb32(pb);
+l = avio_rb32(pb);
+r = avio_rb32(pb);
+
+if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
+av_log(c->fc, AV_LOG_ERROR,
+ "Invalid bounding rectangle coordinates "
+ "%zu,%zu,%zu,%zu\n", l, t, r, b);
+return AVERROR_INVALIDDATA;
+}
+
+if (l || t || r || b)
+projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
+else
+projection = AV_SPHERICAL_EQUIRECTANGULAR;
break;
default:
av_log(c->fc, AV_LOG_ERROR, "Unknown projection type\n");
@@ -4705,6 +4723,19 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
sc->spherical->pitch = pitch;
sc->spherical->roll = roll;
+sc->spherical->padding = padding;
+
+if (projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
+/* conversion from 0.32 coordinates to pixels */
+size_t orig_width = (size_t) sc->width * UINT32_MAX / (UINT32_MAX -
r - l);
+size_t orig_height = (size_t) sc->height * UINT32_MAX / (UINT32_MAX -
b - t);
+
+/* add a (UINT32_MAX - 1) to round up integer division */
+sc->spherical->bound_left = (orig_width * l + UINT32_MAX - 1) /
UINT32_MAX;
+sc->spherical->bound_top= (orig_height * t + UINT32_MAX - 1) /
UINT32_MAX;
+sc->spherical->bound_right = orig_width - sc->width -
sc->spherical->bound_left;
+sc->spherical->bound_bottom = orig_height - sc->height -
sc->spherical->bound_top;
+}
return 0;
}
@@ -4763,6 +4794,26 @@ static int mov_parse_uuid_spherical(MOVStreamContext
*sc, AVIOContext *pb, size_
val = av_stristr(buffer, "");
if (val)
sc->spherical->roll = strtol(val, NULL, 10) * (1 << 16);
+
+/* tiling */
+val = av_stristr(buffer, "");
+if (val)
+sc->spherical->bound_left = strtol(val, NULL, 10);
+val = av_stristr(buffer, "");
+if (val)
+sc->spherical->bound_top = strtol(val, NULL, 10);
+val = av_stristr(buffer, "");
+if (val)
+sc->spherical->bound_right =
+sc->width - sc->spherical->bound_left - strtol(val, NULL, 10);
+val = av_stristr(buffer, "");
+if (val)
+sc->spherical->bound_bottom =
+sc->height - sc->spherical->bound_top - strtol(val, NULL, 10);
+
+if (sc->spherical->bound_left || sc->spherical->bound_top ||
+sc->spherical->bound_right || sc->spherical->bound_bottom)
+sc->spherical->projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
}
out:
diff --git a/tests/ref/fate/mov-spherical-mono
b/tests/ref/fate/mov-spherical-mono
index 8048aff..a70d879 100644
--- a/tests/ref/fate/mov-spherical-mono
+++ b/tests/ref/fate/mov-spherical-mono
@@ -8,7 +8,11 @@ inverted=0
[SIDE_DATA]
side_data_type=Spherical Mapping
side_data_size=56
-projection=equirectangular
+projection=tiled equirectangular
+bound_left=148
+bound_top=73
+bound_right=147
+bound_bottom=72
yaw=45
pitch=30
roll=15
--
2.10.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 4/4] mov: Validate spherical metadata version
---
As suggested by James.
Please CC.
Vittorio
libavformat/mov.c | 26 ++
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index a1774b3..2efd51e 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4623,7 +4623,7 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
{
AVStream *st;
MOVStreamContext *sc;
-int size;
+int size, version;
int32_t yaw, pitch, roll;
size_t l, t, r, b;
size_t padding = 0;
@@ -4650,7 +4650,13 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
av_log(c->fc, AV_LOG_ERROR, "Missing spherical video header\n");
return 0;
}
-avio_skip(pb, 4); /* version + flags */
+version = avio_r8(pb);
+if (version != 0) {
+av_log(c->fc, AV_LOG_WARNING, "Unknown spherical version %d\n",
+ version);
+return 0;
+}
+avio_skip(pb, 3); /* flags */
avio_skip(pb, size - 12); /* metadata_source */
size = avio_rb32(pb);
@@ -4672,7 +4678,13 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
av_log(c->fc, AV_LOG_ERROR, "Missing projection header box\n");
return 0;
}
-avio_skip(pb, 4); /* version + flags */
+version = avio_r8(pb);
+if (version != 0) {
+av_log(c->fc, AV_LOG_WARNING, "Unknown spherical version %d\n",
+ version);
+return 0;
+}
+avio_skip(pb, 3); /* flags */
/* 16.16 fixed point */
yaw = avio_rb32(pb);
@@ -4684,7 +4696,13 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
return AVERROR_INVALIDDATA;
tag = avio_rl32(pb);
-avio_skip(pb, 4); /* version + flags */
+version = avio_r8(pb);
+if (version != 0) {
+av_log(c->fc, AV_LOG_WARNING, "Unknown spherical version %d\n",
+ version);
+return 0;
+}
+avio_skip(pb, 3); /* flags */
switch (tag) {
case MKTAG('c','b','m','p'):
projection = AV_SPHERICAL_CUBEMAP;
--
2.10.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] h264: Correctly initialize interlaced_frame if tff is set
On Tue, Feb 14, 2017 at 6:21 PM, Michael Niedermayer
wrote:
> On Sat, Feb 11, 2017 at 02:56:32AM +0100, Michael Niedermayer wrote:
>> On Fri, Feb 10, 2017 at 05:21:00PM -0500, Vittorio Giovara wrote:
>> > In particular cases, it is possible to initialize top_field_first
>> > but not interlaced_frame. Make sure to correctly tag a frame
>> > as interlaced when this happens.
>> >
>> > Signed-off-by: Vittorio Giovara
>> > ---
>> > Please CC.
>> > Vittorio
>> >
>> > libavcodec/h264_slice.c | 13 -
>> > 1 file changed, 8 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
>> > index 91a3b25..eeb5202 100644
>> > --- a/libavcodec/h264_slice.c
>> > +++ b/libavcodec/h264_slice.c
>> > @@ -1174,20 +1174,23 @@ static int h264_export_frame_props(H264Context *h)
>> >
>> > if (cur->field_poc[0] != cur->field_poc[1]) {
>> > /* Derive top_field_first from field pocs. */
>> > -cur->f->top_field_first = cur->field_poc[0] < cur->field_poc[1];
>> > +cur->f->interlaced_frame =
>> > +cur->f->top_field_first = cur->field_poc[0] < cur->field_poc[1];
>>
>> this looks like it would set interlaced_frame = 0 if
>> cur->field_poc[0] > cur->field_poc[1];
>>
>> also, do you have a sample that is affected by this ?
>
> thx for the sample, ive pushed a different fix
> does that fix this completely or is some issue remaining ?
Looks good, thanks for the fix.
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] spherical: Add tiled equirectangular type and projection-specific properties
On Wed, Feb 15, 2017 at 8:52 AM, James Almer wrote: > On 2/14/2017 11:20 PM, Vittorio Giovara wrote: >> On Tue, Feb 14, 2017 at 6:54 PM, James Almer wrote: >>> On 2/14/2017 5:52 PM, Vittorio Giovara wrote: >>>> On Fri, Feb 10, 2017 at 6:25 PM, Michael Niedermayer >>>> wrote: >>>>> On Fri, Feb 10, 2017 at 04:11:43PM -0500, Vittorio Giovara wrote: >>>>>> Signed-off-by: Vittorio Giovara >>>>>> --- >>>>>> This should help not losing details over muxing and allows >>>>>> callers to get additional information in a clean manner. >>>>>> >>>>>> Please keep me in CC. >>>>>> Vittorio >>>>>> >>>>>> doc/APIchanges| 5 + >>>>>> ffprobe.c | 11 -- >>>>>> libavformat/dump.c| 10 + >>>>>> libavutil/spherical.h | 56 >>>>>> +++ >>>>>> libavutil/version.h | 2 +- >>>>>> 5 files changed, 81 insertions(+), 3 deletions(-) >>>>> >>>>> breaks fate >>>>> >>>>> --- ./tests/ref/fate/matroska-spherical-mono2017-02-10 >>>>> 23:43:51.993432371 +0100 >>>>> +++ tests/data/fate/matroska-spherical-mono 2017-02-11 >>>>> 00:24:10.297483318 +0100 >>>>> @@ -7,7 +7,7 @@ >>>>> [/SIDE_DATA] >>>>> [SIDE_DATA] >>>>> side_data_type=Spherical Mapping >>>>> -side_data_size=16 >>>>> +side_data_size=56 >>>>> projection=equirectangular >>>>> yaw=45 >>>>> pitch=30 >>>>> Test matroska-spherical-mono failed. Look at >>>>> tests/data/fate/matroska-spherical-mono.err for details. >>>>> make: *** [fate-matroska-spherical-mono] Error 1 >>>> >>>> Ah I didn't notice, it is fixed in the next commit, but I'll amend this >>>> one too. >>>> >>>> >>>> I didn't see any comment/discussion, should I assume it is ok? >>>> Please CC, thank you. >>> >>> These are a lot of projection specific fields. It worries me as the >>> spec may change in the future with new fields added or existing >>> fields changing purpose. Not to mention the Mesh projection, which >>> has like fifty specific fields of its own. >> >> Even if the spec change (which at this point would be a terrible >> terrible thing to do) there are now files in the wild and software >> that have adopted this draft, so we would have to support this anyway. > > If the spec changes, it will be the contents of the equi/cbmp/mesh. > By exporting them raw as extradata, said changes in the spec would > require no changes to our implementation. If the spec changes in a non-backward compatible way, the API is the least of our problems :-) -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] spherical: Add tiled equirectangular type and projection-specific properties
On Tue, Feb 14, 2017 at 6:54 PM, James Almer wrote: > On 2/14/2017 5:52 PM, Vittorio Giovara wrote: >> On Fri, Feb 10, 2017 at 6:25 PM, Michael Niedermayer >> wrote: >>> On Fri, Feb 10, 2017 at 04:11:43PM -0500, Vittorio Giovara wrote: >>>> Signed-off-by: Vittorio Giovara >>>> --- >>>> This should help not losing details over muxing and allows >>>> callers to get additional information in a clean manner. >>>> >>>> Please keep me in CC. >>>> Vittorio >>>> >>>> doc/APIchanges| 5 + >>>> ffprobe.c | 11 -- >>>> libavformat/dump.c| 10 + >>>> libavutil/spherical.h | 56 >>>> +++ >>>> libavutil/version.h | 2 +- >>>> 5 files changed, 81 insertions(+), 3 deletions(-) >>> >>> breaks fate >>> >>> --- ./tests/ref/fate/matroska-spherical-mono2017-02-10 >>> 23:43:51.993432371 +0100 >>> +++ tests/data/fate/matroska-spherical-mono 2017-02-11 >>> 00:24:10.297483318 +0100 >>> @@ -7,7 +7,7 @@ >>> [/SIDE_DATA] >>> [SIDE_DATA] >>> side_data_type=Spherical Mapping >>> -side_data_size=16 >>> +side_data_size=56 >>> projection=equirectangular >>> yaw=45 >>> pitch=30 >>> Test matroska-spherical-mono failed. Look at >>> tests/data/fate/matroska-spherical-mono.err for details. >>> make: *** [fate-matroska-spherical-mono] Error 1 >> >> Ah I didn't notice, it is fixed in the next commit, but I'll amend this one >> too. >> >> >> I didn't see any comment/discussion, should I assume it is ok? >> Please CC, thank you. > > These are a lot of projection specific fields. It worries me as the > spec may change in the future with new fields added or existing > fields changing purpose. Not to mention the Mesh projection, which > has like fifty specific fields of its own. Even if the spec change (which at this point would be a terrible terrible thing to do) there are now files in the wild and software that have adopted this draft, so we would have to support this anyway. > Wouldn't it be a better idea to export the binary data of the > equi/cbmp/mesh boxes into an extradata-like field in the > AVSphericalMapping struct, and let the downstream application parse > it instead? No I don't think so, lavf is an abstraction layer and one of its tasks is to provide a (simple?) unified entry layer. and letting the user parse binary data is IMO bad design and very fragile. Also it's not impossible that another standard for tagging spherical metadata emerges in the future: the current API could very easily wrap it, while exporting the binary entry would be too specification-specific and it would be tied too heavily on the google draft. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] spherical: Add tiled equirectangular type and projection-specific properties
On Tue, Feb 14, 2017 at 4:54 PM, Michael Niedermayer wrote: > On Tue, Feb 14, 2017 at 03:52:39PM -0500, Vittorio Giovara wrote: >> On Fri, Feb 10, 2017 at 6:25 PM, Michael Niedermayer >> wrote: >> > On Fri, Feb 10, 2017 at 04:11:43PM -0500, Vittorio Giovara wrote: >> >> Signed-off-by: Vittorio Giovara >> >> --- >> >> This should help not losing details over muxing and allows >> >> callers to get additional information in a clean manner. >> >> >> >> Please keep me in CC. >> >> Vittorio >> >> >> >> doc/APIchanges| 5 + >> >> ffprobe.c | 11 -- >> >> libavformat/dump.c| 10 + >> >> libavutil/spherical.h | 56 >> >> +++ >> >> libavutil/version.h | 2 +- >> >> 5 files changed, 81 insertions(+), 3 deletions(-) >> > >> > breaks fate >> > >> > --- ./tests/ref/fate/matroska-spherical-mono2017-02-10 >> > 23:43:51.993432371 +0100 >> > +++ tests/data/fate/matroska-spherical-mono 2017-02-11 >> > 00:24:10.297483318 +0100 >> > @@ -7,7 +7,7 @@ >> > [/SIDE_DATA] >> > [SIDE_DATA] >> > side_data_type=Spherical Mapping >> > -side_data_size=16 >> > +side_data_size=56 >> > projection=equirectangular >> > yaw=45 >> > pitch=30 >> > Test matroska-spherical-mono failed. Look at >> > tests/data/fate/matroska-spherical-mono.err for details. >> > make: *** [fate-matroska-spherical-mono] Error 1 >> >> Ah I didn't notice, it is fixed in the next commit, but I'll amend this one >> too. > > maybe iam missing some patch but > it still fails here with all 3 patches of the patchset: ops sorry you're right, i forgot to update the mkv test amended locally -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] spherical: Add tiled equirectangular type and projection-specific properties
On Fri, Feb 10, 2017 at 6:25 PM, Michael Niedermayer wrote: > On Fri, Feb 10, 2017 at 04:11:43PM -0500, Vittorio Giovara wrote: >> Signed-off-by: Vittorio Giovara >> --- >> This should help not losing details over muxing and allows >> callers to get additional information in a clean manner. >> >> Please keep me in CC. >> Vittorio >> >> doc/APIchanges| 5 + >> ffprobe.c | 11 -- >> libavformat/dump.c| 10 + >> libavutil/spherical.h | 56 >> +++ >> libavutil/version.h | 2 +- >> 5 files changed, 81 insertions(+), 3 deletions(-) > > breaks fate > > --- ./tests/ref/fate/matroska-spherical-mono2017-02-10 23:43:51.993432371 > +0100 > +++ tests/data/fate/matroska-spherical-mono 2017-02-11 00:24:10.297483318 > +0100 > @@ -7,7 +7,7 @@ > [/SIDE_DATA] > [SIDE_DATA] > side_data_type=Spherical Mapping > -side_data_size=16 > +side_data_size=56 > projection=equirectangular > yaw=45 > pitch=30 > Test matroska-spherical-mono failed. Look at > tests/data/fate/matroska-spherical-mono.err for details. > make: *** [fate-matroska-spherical-mono] Error 1 Ah I didn't notice, it is fixed in the next commit, but I'll amend this one too. I didn't see any comment/discussion, should I assume it is ok? Please CC, thank you. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] h264: Correctly initialize interlaced_frame if tff is set
In particular cases, it is possible to initialize top_field_first
but not interlaced_frame. Make sure to correctly tag a frame
as interlaced when this happens.
Signed-off-by: Vittorio Giovara
---
Please CC.
Vittorio
libavcodec/h264_slice.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index 91a3b25..eeb5202 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -1174,20 +1174,23 @@ static int h264_export_frame_props(H264Context *h)
if (cur->field_poc[0] != cur->field_poc[1]) {
/* Derive top_field_first from field pocs. */
-cur->f->top_field_first = cur->field_poc[0] < cur->field_poc[1];
+cur->f->interlaced_frame =
+cur->f->top_field_first = cur->field_poc[0] < cur->field_poc[1];
} else {
if (sps->pic_struct_present_flag) {
/* Use picture timing SEI information. Even if it is a
* information of a past frame, better than nothing. */
if (h->sei.picture_timing.pic_struct == SEI_PIC_STRUCT_TOP_BOTTOM
||
-h->sei.picture_timing.pic_struct ==
SEI_PIC_STRUCT_TOP_BOTTOM_TOP)
-cur->f->top_field_first = 1;
-else
+h->sei.picture_timing.pic_struct ==
SEI_PIC_STRUCT_TOP_BOTTOM_TOP) {
+cur->f->interlaced_frame =
+cur->f->top_field_first = 1;
+} else
cur->f->top_field_first = 0;
} else if (cur->f->interlaced_frame) {
/* Default to top field first when pic_struct_present_flag
* is not set but interlaced frame detected */
-cur->f->top_field_first = 1;
+cur->f->interlaced_frame =
+cur->f->top_field_first = 1;
} else {
/* Most likely progressive */
cur->f->top_field_first = 0;
--
2.10.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/3] mkv: Export bounds and padding from spherical metadata
---
Although quite math-heavy, I saw little value in having a single parsing
function, and since it requires stream or demuxer specific information
I preferred to keep the two separate.
Please keep me in CC.
Vittorio
libavformat/matroskadec.c | 34 +-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 7223e94..dc1cd62 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1913,15 +1913,32 @@ static int mkv_parse_video_projection(AVStream *st,
const MatroskaTrack *track)
AVSphericalMapping *spherical;
enum AVSphericalProjection projection;
size_t spherical_size;
+size_t l, t, r, b;
+size_t padding = 0;
int ret;
+GetByteContext gb;
+
+bytestream2_init(&gb, track->video.projection.private.data,
+ track->video.projection.private.size);
switch (track->video.projection.type) {
case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
-projection = AV_SPHERICAL_EQUIRECTANGULAR;
+if (track->video.projection.private.size == 0)
+projection = AV_SPHERICAL_EQUIRECTANGULAR;
+else {
+projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
+bytestream2_skip(&gb, 4); // version + flags
+t = bytestream2_get_be32(&gb);
+b = bytestream2_get_be32(&gb);
+l = bytestream2_get_be32(&gb);
+r = bytestream2_get_be32(&gb);
+}
break;
case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP:
if (track->video.projection.private.size < 4)
return AVERROR_INVALIDDATA;
+bytestream2_skip(&gb, 4); // layout
+padding = bytestream2_get_be32(&gb);
projection = AV_SPHERICAL_CUBEMAP;
break;
default:
@@ -1937,6 +1954,21 @@ static int mkv_parse_video_projection(AVStream *st,
const MatroskaTrack *track)
spherical->pitch = (int32_t)(track->video.projection.pitch * (1 << 16));
spherical->roll = (int32_t)(track->video.projection.roll * (1 << 16));
+spherical->padding = padding;
+
+if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
+/* conversion from 0.32 coordinates to pixels */
+uint32_t max_coord = (uint32_t) -1;
+size_t orig_width = (size_t) track->video.pixel_width * max_coord /
(max_coord - r - l);
+size_t orig_height = (size_t) track->video.pixel_height * max_coord /
(max_coord - b - t);
+
+/* add a (max_coord - 1) to round up integer division */
+spherical->left_bound = (orig_width * l + max_coord - 1) /
max_coord;
+spherical->top_bound= (orig_height * t + max_coord - 1) /
max_coord;
+spherical->right_bound = orig_width - track->video.pixel_width -
spherical->left_bound;
+spherical->bottom_bound = orig_height - track->video.pixel_height -
spherical->top_bound;
+}
+
ret = av_stream_add_side_data(st, AV_PKT_DATA_SPHERICAL, (uint8_t
*)spherical,
spherical_size);
if (ret < 0) {
--
2.10.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/3] mov: Export bounds and padding from spherical metadata
Update the fate test as needed.
---
Please keep me in CC.
Vittorio
libavformat/mov.c | 46 ++-
tests/ref/fate/mov-spherical-mono | 8 +--
2 files changed, 51 insertions(+), 3 deletions(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index ca49786..a8900c1 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4625,6 +4625,8 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
MOVStreamContext *sc;
int size;
int32_t yaw, pitch, roll;
+size_t l, t, r, b;
+size_t padding = 0;
uint32_t tag;
enum AVSphericalProjection projection;
@@ -4686,9 +4688,17 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
switch (tag) {
case MKTAG('c','b','m','p'):
projection = AV_SPHERICAL_CUBEMAP;
+padding = avio_rb32(pb);
break;
case MKTAG('e','q','u','i'):
-projection = AV_SPHERICAL_EQUIRECTANGULAR;
+t = avio_rb32(pb);
+b = avio_rb32(pb);
+l = avio_rb32(pb);
+r = avio_rb32(pb);
+if (l || t || r || b)
+projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
+else
+projection = AV_SPHERICAL_EQUIRECTANGULAR;
break;
default:
av_log(c->fc, AV_LOG_ERROR, "Unknown projection type\n");
@@ -4705,6 +4715,20 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
sc->spherical->pitch = pitch;
sc->spherical->roll = roll;
+sc->spherical->padding = padding;
+
+if (projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
+/* conversion from 0.32 coordinates to pixels */
+uint32_t max_coord = (uint32_t) -1;
+size_t orig_width = (size_t) sc->width * max_coord / (max_coord - r
- l);
+size_t orig_height = (size_t) sc->height * max_coord / (max_coord - b
- t);
+
+/* add a (max_coord - 1) to round up integer division */
+sc->spherical->left_bound = (orig_width * l + max_coord - 1) /
max_coord;
+sc->spherical->top_bound= (orig_height * t + max_coord - 1) /
max_coord;
+sc->spherical->right_bound = orig_width - sc->width -
sc->spherical->left_bound;
+sc->spherical->bottom_bound = orig_height - sc->height -
sc->spherical->top_bound;
+}
return 0;
}
@@ -4763,6 +4787,26 @@ static int mov_parse_uuid_spherical(MOVStreamContext
*sc, AVIOContext *pb, size_
val = av_stristr(buffer, "");
if (val)
sc->spherical->roll = strtol(val, NULL, 10) * (1 << 16);
+
+/* tiling */
+val = av_stristr(buffer, "");
+if (val)
+sc->spherical->left_bound = strtol(val, NULL, 10);
+val = av_stristr(buffer, "");
+if (val)
+sc->spherical->top_bound = strtol(val, NULL, 10);
+val = av_stristr(buffer, "");
+if (val)
+sc->spherical->right_bound =
+sc->width - sc->spherical->left_bound - strtol(val, NULL, 10);
+val = av_stristr(buffer, "");
+if (val)
+sc->spherical->bottom_bound =
+sc->height - sc->spherical->top_bound - strtol(val, NULL, 10);
+
+if (sc->spherical->left_bound || sc->spherical->top_bound ||
+sc->spherical->right_bound || sc->spherical->bottom_bound)
+sc->spherical->projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
}
out:
diff --git a/tests/ref/fate/mov-spherical-mono
b/tests/ref/fate/mov-spherical-mono
index 9f4b4f8..a1365c4 100644
--- a/tests/ref/fate/mov-spherical-mono
+++ b/tests/ref/fate/mov-spherical-mono
@@ -7,8 +7,12 @@ inverted=0
[/SIDE_DATA]
[SIDE_DATA]
side_data_type=Spherical Mapping
-side_data_size=16
-projection=equirectangular
+side_data_size=56
+projection=tiled equirectangular
+left_bound=148
+top_bound=73
+right_bound=147
+bottom_bound=72
yaw=45
pitch=30
roll=15
--
2.10.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/3] spherical: Add tiled equirectangular type and projection-specific properties
Signed-off-by: Vittorio Giovara
---
This should help not losing details over muxing and allows
callers to get additional information in a clean manner.
Please keep me in CC.
Vittorio
doc/APIchanges| 5 +
ffprobe.c | 11 --
libavformat/dump.c| 10 +
libavutil/spherical.h | 56 +++
libavutil/version.h | 2 +-
5 files changed, 81 insertions(+), 3 deletions(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index 8bca71e..8268295 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,11 @@ libavutil: 2015-08-28
API changes, most recent first:
+2017-02-10 - xxx - lavu 55.47.100 / 55.31.0 - spherical.h
+ Add AV_SPHERICAL_EQUIRECTANGULAR_TILE, and projection-specific properties
+ (left_bound, top_bound, right_bound, bottom_bound, padding) to
+ AVSphericalMapping.
+
2017-01-31 - xxx - lavu 55.46.100 / 55.20.0 - cpu.h
Add AV_CPU_FLAG_SSSE3SLOW.
diff --git a/ffprobe.c b/ffprobe.c
index 046f080..dafac56 100644
--- a/ffprobe.c
+++ b/ffprobe.c
@@ -1788,9 +1788,16 @@ static void print_pkt_side_data(WriterContext *w,
const AVSphericalMapping *spherical = (AVSphericalMapping
*)sd->data;
if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR)
print_str("projection", "equirectangular");
-else if (spherical->projection == AV_SPHERICAL_CUBEMAP)
+else if (spherical->projection == AV_SPHERICAL_CUBEMAP) {
print_str("projection", "cubemap");
-else
+print_int("padding", spherical->padding);
+} else if (spherical->projection ==
AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
+print_str("projection", "tiled equirectangular");
+print_int("left_bound", spherical->left_bound);
+print_int("top_bound", spherical->top_bound);
+print_int("right_bound", spherical->right_bound);
+print_int("bottom_bound", spherical->bottom_bound);
+} else
print_str("projection", "unknown");
print_int("yaw", (double) spherical->yaw / (1 << 16));
diff --git a/libavformat/dump.c b/libavformat/dump.c
index d9aa3af..6fbbd5a 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -357,6 +357,8 @@ static void dump_spherical(void *ctx, AVPacketSideData *sd)
av_log(ctx, AV_LOG_INFO, "equirectangular ");
else if (spherical->projection == AV_SPHERICAL_CUBEMAP)
av_log(ctx, AV_LOG_INFO, "cubemap ");
+else if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE)
+av_log(ctx, AV_LOG_INFO, "tiled equirectangular ");
else {
av_log(ctx, AV_LOG_WARNING, "unknown");
return;
@@ -366,6 +368,14 @@ static void dump_spherical(void *ctx, AVPacketSideData *sd)
pitch = ((double)spherical->pitch) / (1 << 16);
roll = ((double)spherical->roll) / (1 << 16);
av_log(ctx, AV_LOG_INFO, "(%f/%f/%f) ", yaw, pitch, roll);
+
+if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
+av_log(ctx, AV_LOG_INFO, "[%zu, %zu, %zu, %zu] ",
+ spherical->left_bound, spherical->top_bound,
+ spherical->right_bound, spherical->bottom_bound);
+} else if (spherical->projection == AV_SPHERICAL_CUBEMAP) {
+av_log(ctx, AV_LOG_INFO, "[pad %zu] ", spherical->padding);
+}
}
static void dump_sidedata(void *ctx, AVStream *st, const char *indent)
diff --git a/libavutil/spherical.h b/libavutil/spherical.h
index eeda625..0cfd0d9 100644
--- a/libavutil/spherical.h
+++ b/libavutil/spherical.h
@@ -63,6 +63,13 @@ enum AVSphericalProjection {
* to the back.
*/
AV_SPHERICAL_CUBEMAP,
+
+/**
+ * Video represents a portion of a sphere mapped on a flat surface
+ * using equirectangular projection. The @ref bounding fields indicate
+ * the position of the current video in a larger surface.
+ */
+AV_SPHERICAL_EQUIRECTANGULAR_TILE,
};
/**
@@ -122,6 +129,55 @@ typedef struct AVSphericalMapping {
/**
* @}
*/
+
+/**
+ * @name Bounding rectangle
+ * @anchor bounding
+ * @{
+ * These fields indicate the location of the current tile, and where
+ * it should be mapped relative to the original surface.
+ *
+ * @code{.unparsed}
+ * ++--+
+ * ||top_bound |
+ * |++ |
+ * | left_bound |tile| |
+ * +<-->||<--->+right_bound
+ * |++ |
+ * ||
Re: [FFmpeg-devel] [PATCH] mov, matroskadec : Allow matroskadec & mov to share spherical parsing logic.
On Wed, Feb 8, 2017 at 9:35 AM, James Almer wrote: > On 2/8/2017 10:47 AM, Vittorio Giovara wrote: >> On Mon, Feb 6, 2017 at 4:59 PM, Aaron Colwell wrote: >>> - Extracts common spherical metadata parsing logic. >>> - Adds checks to enforce that only non-tiled equirect & non-padded cubemaps >>> are accepted. >> >> Hi Aaron, >> this patch basically ignores all my comments, so I'm not very happy with it. >> >> I believe the biggest complaint of mine is that this implementation >> only works with the ffmpeg command line and leaves users in the dark, >> in addition to everything I said in the other thread. There are other >> cosmetics oddities (such as adding a *mov* dependency to the >> *matroska* code > > The matroska elements contain the spherical data in the same structure > as they are in mov files. Much like we're including oggdec.h or flac.h > to parse vorbis comment and flac headers, including a new header to > parse an specific kind of mov atom available in other formats should be > ok. > > After all, the alternative would be duplicate the parsing code in two > demuxers. ..which is what we do for most of the other places (replaygain, stereo3d, mastering display and so on and on).. Besides, that was just a cosmetic comment, if the code is to be shared among formats (which I believe is totally unnecessary for _just two_ demuxers) dropping the 'mov' prefix from the file would do, in my opinion. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] imgutils: Document av_image_get_buffer_size()
On Tue, Feb 7, 2017 at 5:30 PM, Michael Niedermayer wrote: > and the forgotten-CC mail > > On Tue, Feb 07, 2017 at 11:28:21PM +0100, Michael Niedermayer wrote: >> On Tue, Feb 07, 2017 at 10:01:52AM -0500, Vittorio Giovara wrote: >> > --- >> > libavutil/imgutils.h | 7 ++- >> > 1 file changed, 6 insertions(+), 1 deletion(-) >> > >> > diff --git a/libavutil/imgutils.h b/libavutil/imgutils.h >> > index 67063a2..ac1bcb8 100644 >> > --- a/libavutil/imgutils.h >> > +++ b/libavutil/imgutils.h >> > @@ -169,7 +169,12 @@ int av_image_fill_arrays(uint8_t *dst_data[4], int >> > dst_linesize[4], >> > * Return the size in bytes of the amount of data required to store an >> > * image with the given parameters. >> > * >> > - * @param[in] align the assumed linesize alignment >> > + * @param pix_fmt the pixel format of the image >> > + * @param widththe width of the image in pixels >> > + * @param height the height of the image in pixels >> > + * @param alignthe assumed linesize alignment >> >> > + * @return the size in bytes required for src, a negative error code >> > + * in case of failure >> >> There is no "src" in this function or its documentation Oh good catch thanks. I'll replace the @return line to * @return the buffer size in bytes, a negative error code in case of failure -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mov, matroskadec : Allow matroskadec & mov to share spherical parsing logic.
On Mon, Feb 6, 2017 at 4:59 PM, Aaron Colwell wrote: > - Extracts common spherical metadata parsing logic. > - Adds checks to enforce that only non-tiled equirect & non-padded cubemaps > are accepted. Hi Aaron, this patch basically ignores all my comments, so I'm not very happy with it. I believe the biggest complaint of mine is that this implementation only works with the ffmpeg command line and leaves users in the dark, in addition to everything I said in the other thread. There are other cosmetics oddities (such as adding a *mov* dependency to the *matroska* code, or mixing code move with new functionality) which are practices that should be avoided. If you don't mind, I'll try to work on something that should finally settle this and provide a patch. Cheers -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] imgutils: Document av_image_get_buffer_size()
--- libavutil/imgutils.h | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libavutil/imgutils.h b/libavutil/imgutils.h index 67063a2..ac1bcb8 100644 --- a/libavutil/imgutils.h +++ b/libavutil/imgutils.h @@ -169,7 +169,12 @@ int av_image_fill_arrays(uint8_t *dst_data[4], int dst_linesize[4], * Return the size in bytes of the amount of data required to store an * image with the given parameters. * - * @param[in] align the assumed linesize alignment + * @param pix_fmt the pixel format of the image + * @param widththe width of the image in pixels + * @param height the height of the image in pixels + * @param alignthe assumed linesize alignment + * @return the size in bytes required for src, a negative error code + * in case of failure */ int av_image_get_buffer_size(enum AVPixelFormat pix_fmt, int width, int height, int align); -- 2.10.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] matroskaenc: Add support for writing video projection.
On Fri, Feb 3, 2017 at 5:57 PM, Aaron Colwell wrote: > I still think you don't understand what these fields do given what you say > here. Yes there is a little more math. At the end of the day all these > fields do is specify a the min & max for the latitude & longitude. This > essentially translates to adding scale factors and offsets in your shader or > something similar in your 3D geometry creation logic. I get it if > implementations don't want to do this small bit of extra work, but saying > this is a different type seems strange because you wouldn't do this when > talking about cropped 2D images. If I don't understand these fields, maybe the specification could do a better job at explaining what they are for ;) I am aware that the final projection is the same, but the actual problem is that if we don't introduce a new type we would be conveying a different semantics to a single projection type. In other words we require applications to behave differently according to two different fields (the projection type and the offset fields) rather than a single one. So yes, the projection is the same, the usecase is different, the app behaviour is different, the final processing is different -- I think that all this warrants a separate type. If I still didn't get my point across, let's try with an example: an application that does not support the tiled equirectangular projection, with a separate type it can immediately discern that it is an unsupported type and inform the user, with a single type, instead, it has to sort-of-implement the tiling and check for fields that should not matter. Another example: look at AVStereo3DType, there is a side-by-side and a side-by-side-quincunx : an application that does not support quincux can just abort and notify the user, if they were two separate fields, you could have applications that do not support quincunx but try to render the side-by-side part (which usually results in a garbage rendering). So I reiterate that in my opinion a separate enum value which "notifies" apps that they should check additional types is the way to go. >> It is too late to change the spec, but I do believe that the usecase >> is different enough to add a new type, in order to not overcomplicate >> the implementation. > > > It feels like you are just moving the problem to the demuxers and muxers > here. Adding a new type means all demuxers will have to contain logic to > generate these different types and all muxers will have to contain logic to > collapse these types back down to a single value. Yes, I would a 100% add more logic to the 2 muxers and 2 demuxers that implement this spec rather than the thousands applications that implement the ffmpeg api. Also the "different logic" is literally an "if" case, if not little else. > I don't really want to keep arguing about this. If folks really want > different types then I'll do it just because I want to get reading and > writing of metadata working end-to-end. I'd like to make a small request to > use the term "cropped equirectagular" instead of "tiled equirectangular" but > I don't feel to strongly about that. Please don't, "cropped" has entirely different meaning, and it's already confusing that you can do bitstream level cropping and surface cropping. If you really hate the term "tiled" you could use "bounded equirectangular" as it is used in the spec. > I suppose this is just a difference in style so I don't feel too strongly > about it. I was just trying to use unions & structs here to make it a little > clearer about which fields are expected to be valid and when. The fields > seemed to be disjoint sets so I was trying to use language features to > convey that. > > I'd also like to float a potential alternative solution. We could just > convey the projection private data as a size and byte array in this struct. > That would allow me to pass the metadata from demuxers to muxers so I can > achieve my goals, and allow actual parsing of the information to be deferred > until someone needs it. How do you feel about this compromise position? Again I don't see any advantage in using your proposal, it makes the code difficult to read and hard to debug. Binary metadata are intrinsically bad in my opinion, for this use case you could just add four fields, and read/write four times. I still have the code I had around when I implemented that. +projection = AV_SPHERICAL_EQUIRECTANGULAR; + +/* 0.32 fixed point */ +t = sc->height * (float) avio_rb32(pb) / ((uint64_t) 1 << 32); +b = sc->height * (float) avio_rb32(pb) / ((uint64_t) 1 << 32); +l = sc->width * (float) avio_rb32(pb) / ((uint64_t) 1 << 32); +r = sc->width * (float) avio_rb32(pb) / ((uint64_t) 1 << 32); [...] +sc->spherical->left_offset = l; +sc->spherical->top_offset= t; +sc->spherical->right_offset = r; +sc->spherical->bottom_offset = b; (and the reverse for writing of course). -- Vittorio ___
Re: [FFmpeg-devel] [PATCH] avcodec/dpxenc: support colour metadata in DPX encoder, fixes ticket #6023
On Fri, Feb 3, 2017 at 2:10 PM, Kieran O Leary wrote:
> Hi Vittorio!
>
> thanks for getting back to me.
>
> On Fri, Feb 3, 2017 at 12:57 PM, Vittorio Giovara
> wrote:
>>
>>
>>
>> Hey Kieran,
>> I think the code looks fine. I am just wondering if we should also
>> offer the possibility to set these flags from the standard context
>> options (-color_trc and others). I'm aware that not all values match
>> or are valid but maybe a small conversion table or extending the main
>> table could be a viable approach. Similarly this could be done for the
>> decoder so that color properties are not lost during a dpx->dpx
>> conversion maybe.
>
>
> That seems to be the general consensus from the replies from James Almer and
> Carl Eugen and it's what i should push towards.
> I added the new values locally to pixfmt.h. I'm thinking that these could be
> called in a similar way to the EXR decoder?
> https://github.com/FFmpeg/FFmpeg/blob/8a1759ad46f05375c957f33049b4592befbcb224/libavcodec/exr.c#L1840
Not sure to which changes you mean, all the values listed by that
commit are already supported by the current
AVColorTransferCharacteristic implementation. Incidentally, I believe
that the codec you point to is a perfect examples of something
libavcodec should not do, color conversion in a decoder: in my opinion
this a task that should be reserved for something external such as
lavfi or ideally lsws.
> In terms of translation tables, could you point me to some simlar code that
> could serve as a starting point for me? The nearest that made sense to me
> seems to be these values in vf_colorpsace.c
> https://github.com/FFmpeg/FFmpeg/blob/master/libavfilter/vf_colorspace.c#L97
> ?
I didn't explain myself well enough, I was mainly suggesting that
rather than having a private option used to tag the final output, you
could *also* read the standard command line option in avctx->color_trc
and use it to tag the output. Since you can't reuse the value because
dpx seem to use a different table, you should translate the value from
AVColorTransferCharacteristic to whatever dpx accepts. In pseudocode
if (priv_trc_opt = "") {
if (avctx->color_trc == AVCOL_BT709)
buf[801] = DPX_BT709
else if (avctx->color_trc == AVCOL_BT601)
buf[801] = DPX_BT601
...
} else
buf[801] = priv_trc_opt
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/dpxenc: support colour metadata in DPX encoder, fixes ticket #6023
On Wed, Feb 1, 2017 at 1:42 PM, Kieran O Leary wrote: > Hello, > > I'm cc'ing Vittorio as I don't think that he's subscribed to the list but > he's contributed to dpxenc.c and recent colorspace filters. The same with > Kate Murray from the Library of Congress who knows a lot more about DPX than > me. Apologies if this is inappropriate. > > I mostly based this patch on other ffmpeg encoders, such as pncenc.c. I'm > not really a C coder, I'm a moving image archivist who needs to be able to > specify colour metadata in DPX for various workflows. Please excuse my > ignorance/mistakes. > > This patch adds documentation and two command line options for the DPX > encoder: > -trc (Transfer Characteristics) and -clr (Colorimetric Specification), which > set colour metadata values in a DPX file. Currently these are hardcoded to > always be 2, aka Linear. Ticket #6023 is related to this, but there have > also been many mailing list posts about this issue: > https://ffmpeg.org/pipermail/ffmpeg-user/2015-March/025630.html > https://ffmpeg.org/pipermail/ffmpeg-user/2015-December/029456.html > > I've kept the default values as 2 (Linear) as this is what was originally in > dpxenc, but I'm not sure of the value of this really. I think that there's > more value in a default of 0 (User-defined) which would just leave the > values unspecified. Or perhaps no value at all! The initial default of 2 for > colorimetric was potentially useless as 2 is listed as 'Not applicable' for > colorimetric specification in SMPTE 268M-2003. > > The values for each of these options are the integers listed in the SMPTE > standards doc: > https://web.archive.org/web/20050706060025/http://www.smpte.org/smpte_store/standards/pdf/s268m.pdf > > Initially I just had one argument that set the Transfer Characteristic and > Colorimetric Specification to the same value, but perhaps some use cases > could require that these values be different? I'm not sure if they ever > would. I have never seen real world files that suggest this but I can edit > this if it seems weird. > > Some of the values from 0-12 are listed as 'Not applicable' for the > colorimetric specification, but I didn't know how to specify just those > numbers (0-1, 4-10) in the patch. Perhaps it's OK to leave it as is, > otherwise hopefully someone can point me to similar code that I can learn > from. Again, apologies for my ignorance. > Hey Kieran, I think the code looks fine. I am just wondering if we should also offer the possibility to set these flags from the standard context options (-color_trc and others). I'm aware that not all values match or are valid but maybe a small conversion table or extending the main table could be a viable approach. Similarly this could be done for the decoder so that color properties are not lost during a dpx->dpx conversion maybe. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] matroskaenc: Add support for writing video projection.
On Thu, Feb 2, 2017 at 9:34 PM, James Almer wrote: > On 1/31/2017 12:40 PM, Aaron Colwell wrote: >> >> >> On Tue, Jan 31, 2017 at 2:12 AM Vittorio Giovara > <mailto:vittorio.giov...@gmail.com>> wrote: >> >> On Sat, Jan 28, 2017 at 4:13 AM, James Almer > <mailto:jamr...@gmail.com>> wrote: >>> On 1/27/2017 11:21 PM, Aaron Colwell wrote: >>>> On Fri, Jan 27, 2017 at 5:46 PM James Almer >>> <mailto:jamr...@gmail.com>> wrote: >>>> >>>> yeah. I'm a little confused why the demuxing code didn't implement this to >>>> begin with. >>> >>> Vittorio (The one that implemented AVSphericalMapping) tried to add this at >>> first, but then removed it because he wasn't sure if he was doing it right. >> >> Hi, >> yes this was included initially but then we found out what those >> fields were really for, and I didn't want to make the users get as >> confused as we were. As a matter of fact Aaron I mentioned this to you >> when I proposed that we probably should have separated the classic >> equi projection from the tiled one in the specification, in order to >> simplify the implementation. >> >> >> Like I said before, it is not a different projection. It is still >> equirectangular and those parameters just crops the projection. It is very >> simple to just verify that the fields are zero if you don't want to support >> the cropped version. Hello, I'm sorry but I heavily disagree. The tiled equirectangular projection is something that cannot be used standalone, you have to do additional mathematics and take into account different files or streams to generate a "normal" or full-frame equirectangular projection. Having a separate type allows to include extensions such as the bounds fields, which can be safely ignored by the every user that do not need a tiled projection. It is too late to change the spec, but I do believe that the usecase is different enough to add a new type, in order to not overcomplicate the implementation. >>>>> I know you're the one behind the spec in question, but wouldn't it be a >>>>> better idea to wait until AVSphericalMapping gets a way to propagate this >>>>> kind of information before adding support for muxing video projection >>>>> elements? Or maybe try to implement it right now... >>>>> >>>> >>>> I'm happy to implement support for the projection specific info. What would >>>> be the best way to proceed. It seems like I could just place a union with >>>> projection specific structs in AVSphericalMapping. I'd also like some >>> >>> I'm CCing Vittorio so he can chim in. I personally have no real preference. >> >> The best way in my opinion is to add a third type, such as >> AV_SPHERICAL_TILED_EQUI, and add the relevant fields in >> AVSphericalMapping, with a clear description about the actual use case >> for them, mentioning that they are used only in format. By the way, >> why do you mention adding a union? I think four plain fields should >> do. >> >> >> I don't think it is worth having the extra enum value for this. All the >> cropped fields do is control how you generate the spherical mesh or control >> the shader used to render the projection. If players don't want to support >> it they can just check to see that all the fields are zero and error out if >> not. Why would you have them check these fields every time, when this can be implicitly determined by the type semantics. I'm pretty sure API users prefer this scenario * check projection type -> if normal_equi -> project it -> if tiled_equi -> read additional data -> project it rather than * check projection type -> if equi -> read additional data -> check if data needs additional processing -> project it, or perform more operations before projecting >> I was suggesting using a union because the projection bounds fields are for >> equirect, and there are different fields for the cubemap & mesh projections. >> I figured that the enum + union of structs might be a reasonable way to >> organize the projection specific fields. This is a structure whose size does not depend on ABI and can be extended as we like, there is no need to separate new fields in such a way as long as they are properly documented, in my opinion. Please keep me in CC. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] matroskaenc: Add support for writing video projection.
On Sat, Jan 28, 2017 at 4:13 AM, James Almer wrote: > On 1/27/2017 11:21 PM, Aaron Colwell wrote: >> On Fri, Jan 27, 2017 at 5:46 PM James Almer wrote: >> >> yeah. I'm a little confused why the demuxing code didn't implement this to >> begin with. > > Vittorio (The one that implemented AVSphericalMapping) tried to add this at > first, but then removed it because he wasn't sure if he was doing it right. Hi, yes this was included initially but then we found out what those fields were really for, and I didn't want to make the users get as confused as we were. As a matter of fact Aaron I mentioned this to you when I proposed that we probably should have separated the classic equi projection from the tiled one in the specification, in order to simplify the implementation. >>> I know you're the one behind the spec in question, but wouldn't it be a >>> better idea to wait until AVSphericalMapping gets a way to propagate this >>> kind of information before adding support for muxing video projection >>> elements? Or maybe try to implement it right now... >>> >> >> I'm happy to implement support for the projection specific info. What would >> be the best way to proceed. It seems like I could just place a union with >> projection specific structs in AVSphericalMapping. I'd also like some > > I'm CCing Vittorio so he can chim in. I personally have no real preference. The best way in my opinion is to add a third type, such as AV_SPHERICAL_TILED_EQUI, and add the relevant fields in AVSphericalMapping, with a clear description about the actual use case for them, mentioning that they are used only in format. By the way, why do you mention adding a union? I think four plain fields should do. Please keep me in CC. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/texturedspenc: Fix indexing in color distribution determination
On Wed, Jan 4, 2017 at 1:06 AM, Michael Niedermayer
wrote:
> On Tue, Jan 03, 2017 at 02:15:20PM +0100, Vittorio Giovara wrote:
>> On Mon, Jan 2, 2017 at 2:00 AM, James Almer wrote:
>> > On 1/1/2017 8:28 PM, Michael Niedermayer wrote:
>> >> Fixes CID1396405
>>
>> What is the CID about?
>
> sorry for not quoting this prviously, it isnt very interresting.
> my patch is based on just this and guesswork, which is why it should
> be tested.
>
> 257for (y = 0; y < 4; y++) {
>assignment: Assigning: x = 4.
>const: At condition x < 4, the value of x must be equal to 4.
>dead_error_condition: The condition x < 4 cannot be true.
> 258for (x = 4; x < 4; x += 4) {
>CID 1396405 (#1 of 1): Logically dead code (DEADCODE)dead_error_begin:
> Execution cannot reach this statement: muv += bp[x * 4 + y * stride];.
Ah, yes that seems wrong, probably a leftover of the code conversion
(from https://github.com/nothings/stb/blob/master/stb_dxt.h#L293-L298)
I believe it should be sufficient to do `for (x = 0; x < 4; x++)` but
I won't be able to test that for a while I'm afraid
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/texturedspenc: Fix indexing in color distribution determination
On Mon, Jan 2, 2017 at 2:00 AM, James Almer wrote: > On 1/1/2017 8:28 PM, Michael Niedermayer wrote: >> Fixes CID1396405 What is the CID about? >> Untested except fate which does not test this, please someone test this > > The encoder seems to need libsnappy, so no FATE test can be added. Only the HAP encoder needs libsnappy, this module has no external dependencies. A possible way to test this is to encode a HAP video using -format option and hap_alpha or hap_q modes and then decode it back with the internal decoder. > CCing Vittorio since he wrote this. He may be able to test and/or > review. Thanks for the CC. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavu: Add AVSphericalMapping type and frame side data
On Tue, Dec 6, 2016 at 10:46 AM, Vittorio Giovara
wrote:
> While no decoder currently exports spherical information, this type
> represents a frame property that has to be passed through from container
> to frames.
>
> Signed-off-by: Vittorio Giovara
> ---
> +/**
> + * @name Initial orientation
> + * @{
> + * There fields describe additional rotations applied to the sphere after
> + * the video frame is mapped onto it. The sphere is rotated around the
> + * viewer, who remains stationary. The order of transformation is always
> + * yaw, followed by pitch, and finally by roll.
> + *
> + * The coordinate system matches the one defined in OpenGL, where the
> + * forward vector (z) is coming out of screen, and it is equivalent to
> + * a rotation matrix of R = r_y(yaw) * r_x(pitch) * r_z(roll).
> + *
> + * A positive yaw points the viewer towards the left.
> + * A positive pitch points the viewer downwards.
> + * A positive roll tilts the viewer to the right.
One more iteration of this description, hopefully final:
* A positive yaw rotates the portion of the sphere in front of the viewer
* toward their right. A positive pitch rotates the portion of the sphere
* in front of the viewer upwards. A positive roll tilts the portion of
* the sphere in front of the viewer to the viewer's right.
Unless objections I'll push the set in the next few days.
Please CC.
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] movenc: Tag files generated with strict experimental with a warning
On Tue, Dec 6, 2016 at 10:50 AM, James Almer wrote: > On 12/2/2016 5:17 PM, James Almer wrote: >> On 12/2/2016 5:00 PM, Vittorio Giovara wrote: >>> This will simplify identifying files that were generated with >>> unfinished/incomplete/non-standard specifications. >>> >>> Signed-off-by: Vittorio Giovara >>> --- >>> libavformat/movenc.c | 2 ++ >>> 1 file changed, 2 insertions(+) >> >> Maybe lavf and lavc should add "experimental" to the container and >> stream's metadata alongside the library version, much like how we're >> adding the name of the encoder alongside the lavc version to the >> stream's metadata. >> >> A warning printed by the muxer not going to help once such a file >> is in the wild. Yeah I was unsure about that -- are you saying that whenever -strict experimental is used we should add this tag regardless? The idea behind this is that in the future, when the standard is consolidated, if there is a file is in the wild encoded with an "experimental" codec/container combination and it is not playing correctly, this tag will make evident that the bug lies within the file and the playback issues are probably due the unfinished state of the specification, rather than in in the modern implementation of the demuxer. In other words, people will be less inclined to hack around their own demuxer to accommodate for broken files. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/4] mov: Export spherical information
On Tue, Dec 6, 2016 at 11:06 AM, James Almer wrote: > On 11/30/2016 9:12 PM, James Almer wrote: >> On 11/30/2016 8:36 PM, Vittorio Giovara wrote: >>> This implements Spherical Video V1 and V2, as described in the >>> spatial-media collection by Google. >>> >>> Signed-off-by: Vittorio Giovara >>> --- >>> This addresses all comments from James. >>> Vittorio >> >> LGTM, at least the V2 part. Seems to follow the spec right now. >> Will trust you on the V1 part :P. Hopefully V1 is going to disappear soon. Thanks for the review, I've sent a minor documentation update which reflects the latest change to the specification, I hope that is fine too. >> I have written a Matroska implementation of this spec, for that >> matter. Will send it after this patchset is committed. Oh cool, do you have or need a file to test (and/or add a fate)? -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavu: Add AVSphericalMapping type and frame side data
While no decoder currently exports spherical information, this type
represents a frame property that has to be passed through from container
to frames.
Signed-off-by: Vittorio Giovara
---
The specification got updated while this was in the works. So I updated
the description of yaw, pitch, roll and added a paragraph about the
coordinate system.
Please CC.
Vittorio
libavutil/Makefile| 2 +
libavutil/avutil.h| 6 +++
libavutil/frame.h | 8 ++-
libavutil/spherical.c | 34 +
libavutil/spherical.h | 136 ++
5 files changed, 185 insertions(+), 1 deletion(-)
create mode 100644 libavutil/spherical.c
create mode 100644 libavutil/spherical.h
diff --git a/libavutil/Makefile b/libavutil/Makefile
index c61ca9c..6474a30 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -63,6 +63,7 @@ HEADERS = adler32.h
\
samplefmt.h \
sha.h \
sha512.h \
+ spherical.h \
stereo3d.h\
threadmessage.h \
time.h\
@@ -140,6 +141,7 @@ OBJS = adler32.o
\
samplefmt.o \
sha.o\
sha512.o \
+ spherical.o \
stereo3d.o \
threadmessage.o \
time.o \
diff --git a/libavutil/avutil.h b/libavutil/avutil.h
index 29dd830..e9aaa03 100644
--- a/libavutil/avutil.h
+++ b/libavutil/avutil.h
@@ -118,6 +118,12 @@
*
* @}
*
+ * @defgroup lavu_video Video related
+ *
+ * @{
+ *
+ * @}
+ *
* @defgroup lavu_audio Audio related
*
* @{
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 8e51361..b450092 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -120,7 +120,13 @@ enum AVFrameSideDataType {
* The GOP timecode in 25 bit timecode format. Data format is 64-bit
integer.
* This is set on the first frame of a GOP that has a temporal reference
of 0.
*/
-AV_FRAME_DATA_GOP_TIMECODE
+AV_FRAME_DATA_GOP_TIMECODE,
+
+/**
+ * The data represents the AVSphericalMapping structure defined in
+ * libavutil/spherical.h.
+ */
+AV_FRAME_DATA_SPHERICAL,
};
enum AVActiveFormatDescription {
diff --git a/libavutil/spherical.c b/libavutil/spherical.c
new file mode 100644
index 000..816452c
--- /dev/null
+++ b/libavutil/spherical.c
@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2016 Vittorio Giovara
+ *
+ * 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 "mem.h"
+#include "spherical.h"
+
+AVSphericalMapping *av_spherical_alloc(size_t *size)
+{
+AVSphericalMapping *spherical = av_mallocz(sizeof(AVSphericalMapping));
+if (!spherical)
+return NULL;
+
+if (size)
+*size = sizeof(*spherical);
+
+return spherical;
+}
diff --git a/libavutil/spherical.h b/libavutil/spherical.h
new file mode 100644
index 000..39db57b
--- /dev/null
+++ b/libavutil/spherical.h
@@ -0,0 +1,136 @@
+/*
+ * Copyright (c) 2016 Vittorio Giovara
+ *
+ * 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 P
Re: [FFmpeg-devel] [PATCH] movenc: Tag files generated with strict experimental with a warning
On Fri, Dec 2, 2016 at 3:00 PM, Vittorio Giovara
wrote:
> This will simplify identifying files that were generated with
> unfinished/incomplete/non-standard specifications.
>
> Signed-off-by: Vittorio Giovara
> ---
> libavformat/movenc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index dc19838..c46bea9 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -5756,6 +5756,7 @@ static int mov_init(AVFormatContext *s)
> FF_COMPLIANCE_EXPERIMENTAL);
> return AVERROR_EXPERIMENTAL;
> }
> +av_dict_set(&s->metadata, "WARNING", "This file was
> generated using an unfinished specification, please don't modify your demuxer
> to support it, should it not work", 0);
> }
> } else if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
> track->timescale = st->codecpar->sample_rate;
> @@ -5802,6 +5803,7 @@ static int mov_init(AVFormatContext *s)
> FF_COMPLIANCE_EXPERIMENTAL);
> return AVERROR_EXPERIMENTAL;
> }
> +av_dict_set(&s->metadata, "WARNING", "This file was
> generated using an unfinished specification, please don't modify your demuxer
> to support it, should it not work", 0);
> }
> } else if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE) {
> track->timescale = st->time_base.den;
> --
Any opinion about this? Should I assume it is fine as is?
Please CC.
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] movenc: Tag files generated with strict experimental with a warning
This will simplify identifying files that were generated with
unfinished/incomplete/non-standard specifications.
Signed-off-by: Vittorio Giovara
---
libavformat/movenc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index dc19838..c46bea9 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -5756,6 +5756,7 @@ static int mov_init(AVFormatContext *s)
FF_COMPLIANCE_EXPERIMENTAL);
return AVERROR_EXPERIMENTAL;
}
+av_dict_set(&s->metadata, "WARNING", "This file was generated
using an unfinished specification, please don't modify your demuxer to support
it, should it not work", 0);
}
} else if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
track->timescale = st->codecpar->sample_rate;
@@ -5802,6 +5803,7 @@ static int mov_init(AVFormatContext *s)
FF_COMPLIANCE_EXPERIMENTAL);
return AVERROR_EXPERIMENTAL;
}
+av_dict_set(&s->metadata, "WARNING", "This file was generated
using an unfinished specification, please don't modify your demuxer to support
it, should it not work", 0);
}
} else if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE) {
track->timescale = st->time_base.den;
--
2.10.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavu: Add JEDEC P22 color primaries
On Thu, Dec 1, 2016 at 3:01 PM, Nicolas George wrote: > Le primidi 11 frimaire, an CCXXV, Vittorio Giovara a écrit : >> Actually we already do, theora, vp8, vp9 and a few others have >> completely different values for most color properties, and we just >> check before initializing the context or frame fields. Since iso/itu >> codecs are the ones that more consistently use all the properties >> everywhere it makes sense that the enum values and field values match >> and imho it's simpler to use. > > It made sense until now, but now they have added a gap, and it becomes > obvious it was not a very good idea to begin with. > > Fortunately, since the architecture is already there for the other > codecs, fixing it should not be much work. Probably about the same > amount as fixing the issues with the gap. I'm not sure about that, right now a gap would just require adding bound checks in exactly 5 different places, whereas restructuring this well-established system will need careful modification for each of the codecs involved, which are quite a few. The real problem to me seems the fact that _NB elements are used outside the library, despite being documented against, so I think it would be more productive to fix those occurrences (especially since this is a user-facing API). -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavu: Add JEDEC P22 color primaries
On Thu, Dec 1, 2016 at 2:22 PM, Nicolas George wrote: > Le primidi 11 frimaire, an CCXXV, Rostislav Pehlivanov a écrit : >> Why do you insist on having a hole in the first place? > > Apparently, the values of the enum are used encode and decode > standardized values in standardized formats like MPEG. > > I think this is a terrible design decision, and I would like to suggest > to change it before it is too late. > > It will lead to all kinds of trouble if we want to support primaries > from other standards with conflicting values, or if the standards > introduce huge gaps in their allocated values. Actually we already do, theora, vp8, vp9 and a few others have completely different values for most color properties, and we just check before initializing the context or frame fields. Since iso/itu codecs are the ones that more consistently use all the properties everywhere it makes sense that the enum values and field values match and imho it's simpler to use. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavu: Add JEDEC P22 color primaries
On Wed, Nov 30, 2016 at 5:48 PM, Ronald S. Bultje wrote: > Hi, > > On Wed, Nov 30, 2016 at 5:26 PM, Andreas Cadhalpun > wrote: >> >> On 30.11.2016 22:55, Ronald S. Bultje wrote: >> > On Wed, Nov 30, 2016 at 4:51 PM, Andreas Cadhalpun < >> > andreas.cadhal...@googlemail.com> wrote: >> > >> >> On 30.11.2016 19:16, Vittorio Giovara wrote: >> >> You can't just add a gap like that. >> >> The current code assumes that the numbers are consecutive, like e.g. >> >> the >> >> naming of AVCOL_PRI_NB suggests. >> > >> > >> > No, we've had gaps in these before. >> >> In AVColorPrimaries? > > > I seemed to remember, looked it up and appeared to be wrong, so scrap that > :) Ok, so I can take care of the "holes" this patch would bring for lavc, would anybody help me with the remaining occurrences (namely in lavf and lavfi)? -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/4] lavc: Add spherical packet side data API
Signed-off-by: Vittorio Giovara
---
No particular change.
Vittorio
ffprobe.c | 13 +
libavcodec/avcodec.h | 8 +++-
libavcodec/avpacket.c | 1 +
libavcodec/utils.c| 1 +
libavformat/dump.c| 30 ++
5 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/ffprobe.c b/ffprobe.c
index 4ed8e06..046f080 100644
--- a/ffprobe.c
+++ b/ffprobe.c
@@ -37,6 +37,7 @@
#include "libavutil/hash.h"
#include "libavutil/opt.h"
#include "libavutil/pixdesc.h"
+#include "libavutil/spherical.h"
#include "libavutil/stereo3d.h"
#include "libavutil/dict.h"
#include "libavutil/intreadwrite.h"
@@ -1783,6 +1784,18 @@ static void print_pkt_side_data(WriterContext *w,
const AVStereo3D *stereo = (AVStereo3D *)sd->data;
print_str("type", av_stereo3d_type_name(stereo->type));
print_int("inverted", !!(stereo->flags & AV_STEREO3D_FLAG_INVERT));
+} else if (sd->type == AV_PKT_DATA_SPHERICAL) {
+const AVSphericalMapping *spherical = (AVSphericalMapping
*)sd->data;
+if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR)
+print_str("projection", "equirectangular");
+else if (spherical->projection == AV_SPHERICAL_CUBEMAP)
+print_str("projection", "cubemap");
+else
+print_str("projection", "unknown");
+
+print_int("yaw", (double) spherical->yaw / (1 << 16));
+print_int("pitch", (double) spherical->pitch / (1 << 16));
+print_int("roll", (double) spherical->roll / (1 << 16));
}
writer_print_section_footer(w);
}
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index e5e7f42..7ac2ada 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1536,7 +1536,13 @@ enum AVPacketSideDataType {
* should be associated with a video stream and containts data in the form
* of the AVMasteringDisplayMetadata struct.
*/
-AV_PKT_DATA_MASTERING_DISPLAY_METADATA
+AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
+
+/**
+ * This side data should be associated with a video stream and corresponds
+ * to the AVSphericalMapping structure.
+ */
+AV_PKT_DATA_SPHERICAL,
};
#define AV_PKT_DATA_QUALITY_FACTOR AV_PKT_DATA_QUALITY_STATS //DEPRECATED
diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index e5a8bdb..24b4efb 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -372,6 +372,7 @@ const char *av_packet_side_data_name(enum
AVPacketSideDataType type)
case AV_PKT_DATA_METADATA_UPDATE:return "Metadata Update";
case AV_PKT_DATA_MPEGTS_STREAM_ID: return "MPEGTS Stream ID";
case AV_PKT_DATA_MASTERING_DISPLAY_METADATA: return "Mastering display
metadata";
+case AV_PKT_DATA_SPHERICAL: return "Spherical Mapping";
}
return NULL;
}
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index d6dca18..89a12c6 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -762,6 +762,7 @@ int ff_init_buffer_info(AVCodecContext *avctx, AVFrame
*frame)
} sd[] = {
{ AV_PKT_DATA_REPLAYGAIN ,AV_FRAME_DATA_REPLAYGAIN },
{ AV_PKT_DATA_DISPLAYMATRIX, AV_FRAME_DATA_DISPLAYMATRIX
},
+{ AV_PKT_DATA_SPHERICAL, AV_FRAME_DATA_SPHERICAL },
{ AV_PKT_DATA_STEREO3D, AV_FRAME_DATA_STEREO3D },
{ AV_PKT_DATA_AUDIO_SERVICE_TYPE,
AV_FRAME_DATA_AUDIO_SERVICE_TYPE },
{ AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
AV_FRAME_DATA_MASTERING_DISPLAY_METADATA },
diff --git a/libavformat/dump.c b/libavformat/dump.c
index cd14625..bcc6a00 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -31,6 +31,7 @@
#include "libavutil/opt.h"
#include "libavutil/avstring.h"
#include "libavutil/replaygain.h"
+#include "libavutil/spherical.h"
#include "libavutil/stereo3d.h"
#include "avformat.h"
@@ -342,6 +343,31 @@ static void dump_mastering_display_metadata(void *ctx,
AVPacketSideData* sd) {
av_q2d(metadata->min_luminance), av_q2d(metadata->max_luminance));
}
+static void dump_spherical(void *ctx, AVPacketSideData *sd)
+{
+AVSphericalMapping *spherical = (AVSphericalMapping *)sd->data;
+double yaw, pitch, roll;
+
+if (sd->size < sizeof(*spherical)) {
+av_log(ctx, AV_LOG_INFO, "invalid data");
+return;
+}
+
+if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR)
+av_log(ctx, AV_LOG_INFO, "equirectangular ");
+else if (spherical-&g
[FFmpeg-devel] [PATCH] fate/mov: Rename a couple of entries to respect the file naming scheme
--- tests/fate/mov.mak | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak index 71c9a1f..f642f1d 100644 --- a/tests/fate/mov.mak +++ b/tests/fate/mov.mak @@ -7,7 +7,7 @@ FATE_MOV = fate-mov-3elist \ fate-mov-2elist-elist1-ends-bframe \ fate-mov-zombie \ fate-mov-aac-2048-priming \ - fate-mp4-init-nonkeyframe \ + fate-mov-init-nonkeyframe \ fate-mov-displaymatrix \ FATE_SAMPLES_AVCONV += $(FATE_MOV) @@ -37,8 +37,8 @@ fate-mov-aac-2048-priming: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_packets - fate-mov-zombie: ffprobe$(PROGSSUF)$(EXESUF) fate-mov-zombie: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_streams -show_packets -show_frames -bitexact -print_format compact $(TARGET_SAMPLES)/mov/white_zombie_scrunch-part.mov -fate-mp4-init-nonkeyframe: ffprobe$(PROGSSUF)$(EXESUF) -fate-mp4-init-nonkeyframe: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_packets -print_format compact -select_streams v $(TARGET_SAMPLES)/mov/mp4-init-nonkeyframe.mp4 +fate-mov-init-nonkeyframe: ffprobe$(PROGSSUF)$(EXESUF) +fate-mov-init-nonkeyframe: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_packets -print_format compact -select_streams v $(TARGET_SAMPLES)/mov/mp4-init-nonkeyframe.mp4 -fate-mp4-displaymatrix: ffprobe$(PROGSSUF)$(EXESUF) +fate-mov-displaymatrix: ffprobe$(PROGSSUF)$(EXESUF) fate-mov-displaymatrix: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_entries stream_side_data_list -select_streams v -v 0 $(TARGET_SAMPLES)/mov/displaymatrix.mov -- 2.10.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 4/4] fate: Add a monoscopic spherical mov test
--- This needs https://www.dropbox.com/s/0lqxkzy16o7fihz/spherical.mov?dl=0 to be placed in $SAMPLES/mov. Vittorio tests/fate/mov.mak| 4 tests/ref/fate/mov-spherical-mono | 16 2 files changed, 20 insertions(+) create mode 100644 tests/ref/fate/mov-spherical-mono diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak index 71c9a1f..32ba72b 100644 --- a/tests/fate/mov.mak +++ b/tests/fate/mov.mak @@ -9,6 +9,7 @@ FATE_MOV = fate-mov-3elist \ fate-mov-aac-2048-priming \ fate-mp4-init-nonkeyframe \ fate-mov-displaymatrix \ + fate-mov-spherical-mono \ FATE_SAMPLES_AVCONV += $(FATE_MOV) @@ -42,3 +43,6 @@ fate-mp4-init-nonkeyframe: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_packets - fate-mp4-displaymatrix: ffprobe$(PROGSSUF)$(EXESUF) fate-mov-displaymatrix: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_entries stream_side_data_list -select_streams v -v 0 $(TARGET_SAMPLES)/mov/displaymatrix.mov + +fate-mov-spherical-mono: ffprobe$(PROGSSUF)$(EXESUF) +fate-mov-spherical-mono: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_entries stream_side_data_list -select_streams v -v 0 $(TARGET_SAMPLES)/mov/spherical.mov diff --git a/tests/ref/fate/mov-spherical-mono b/tests/ref/fate/mov-spherical-mono new file mode 100644 index 000..9f4b4f8 --- /dev/null +++ b/tests/ref/fate/mov-spherical-mono @@ -0,0 +1,16 @@ +[STREAM] +[SIDE_DATA] +side_data_type=Stereo 3D +side_data_size=8 +type=2D +inverted=0 +[/SIDE_DATA] +[SIDE_DATA] +side_data_type=Spherical Mapping +side_data_size=16 +projection=equirectangular +yaw=45 +pitch=30 +roll=15 +[/SIDE_DATA] +[/STREAM] -- 2.10.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/4] mov: Export spherical information
This implements Spherical Video V1 and V2, as described in the
spatial-media collection by Google.
Signed-off-by: Vittorio Giovara
---
This addresses all comments from James.
Vittorio
libavformat/isom.h | 7 ++
libavformat/mov.c | 239 -
2 files changed, 245 insertions(+), 1 deletion(-)
diff --git a/libavformat/isom.h b/libavformat/isom.h
index 02bfedd..0fd9eb0 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -24,6 +24,9 @@
#ifndef AVFORMAT_ISOM_H
#define AVFORMAT_ISOM_H
+#include "libavutil/spherical.h"
+#include "libavutil/stereo3d.h"
+
#include "avio.h"
#include "internal.h"
#include "dv.h"
@@ -177,6 +180,10 @@ typedef struct MOVStreamContext {
int stsd_count;
int32_t *display_matrix;
+AVStereo3D *stereo3d;
+AVSphericalMapping *spherical;
+size_t spherical_size;
+
uint32_t format;
int has_sidx; // If there is an sidx entry for this stream.
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 6fb938a..0b1c182 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -42,6 +42,8 @@
#include "libavutil/aes.h"
#include "libavutil/aes_ctr.h"
#include "libavutil/sha.h"
+#include "libavutil/spherical.h"
+#include "libavutil/stereo3d.h"
#include "libavutil/timecode.h"
#include "libavcodec/ac3tab.h"
#include "libavcodec/flac.h"
@@ -4498,8 +4500,204 @@ static int mov_read_tmcd(MOVContext *c, AVIOContext
*pb, MOVAtom atom)
return 0;
}
+static int mov_read_st3d(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+AVStream *st;
+MOVStreamContext *sc;
+enum AVStereo3DType type;
+int mode;
+
+if (c->fc->nb_streams < 1)
+return 0;
+
+st = c->fc->streams[c->fc->nb_streams - 1];
+sc = st->priv_data;
+
+if (atom.size < 5) {
+av_log(c->fc, AV_LOG_ERROR, "Empty stereoscopic video box\n");
+return AVERROR_INVALIDDATA;
+}
+avio_skip(pb, 4); /* version + flags */
+
+mode = avio_r8(pb);
+switch (mode) {
+case 0:
+type = AV_STEREO3D_2D;
+break;
+case 1:
+type = AV_STEREO3D_TOPBOTTOM;
+break;
+case 2:
+type = AV_STEREO3D_SIDEBYSIDE;
+break;
+default:
+av_log(c->fc, AV_LOG_WARNING, "Unknown st3d mode value %d\n", mode);
+return 0;
+}
+
+sc->stereo3d = av_stereo3d_alloc();
+if (!sc->stereo3d)
+return AVERROR(ENOMEM);
+
+sc->stereo3d->type = type;
+return 0;
+}
+
+static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+AVStream *st;
+MOVStreamContext *sc;
+int size;
+int32_t yaw, pitch, roll;
+uint32_t tag;
+enum AVSphericalProjection projection;
+
+if (c->fc->nb_streams < 1)
+return 0;
+
+st = c->fc->streams[c->fc->nb_streams - 1];
+sc = st->priv_data;
+
+if (atom.size < 8) {
+av_log(c->fc, AV_LOG_ERROR, "Empty spherical video box\n");
+return AVERROR_INVALIDDATA;
+}
+
+size = avio_rb32(pb);
+if (size > atom.size)
+return AVERROR_INVALIDDATA;
+
+tag = avio_rl32(pb);
+if (tag != MKTAG('s','v','h','d')) {
+av_log(c->fc, AV_LOG_ERROR, "Missing spherical video header\n");
+return 0;
+}
+avio_skip(pb, 4); /* version + flags */
+avio_skip(pb, avio_r8(pb)); /* metadata_source */
+
+size = avio_rb32(pb);
+if (size > atom.size)
+return AVERROR_INVALIDDATA;
+
+tag = avio_rl32(pb);
+if (tag != MKTAG('p','r','o','j')) {
+av_log(c->fc, AV_LOG_ERROR, "Missing projection box\n");
+return 0;
+}
+
+size = avio_rb32(pb);
+if (size > atom.size)
+return AVERROR_INVALIDDATA;
+
+tag = avio_rl32(pb);
+if (tag != MKTAG('p','r','h','d')) {
+av_log(c->fc, AV_LOG_ERROR, "Missing projection header box\n");
+return 0;
+}
+avio_skip(pb, 4); /* version + flags */
+
+/* 16.16 fixed point */
+yaw = avio_rb32(pb);
+pitch = avio_rb32(pb);
+roll = avio_rb32(pb);
+
+size = avio_rb32(pb);
+if (size > atom.size)
+return AVERROR_INVALIDDATA;
+
+tag = avio_rl32(pb);
+avio_skip(pb, 4); /* version + flags */
+switch (tag) {
+case MKTAG('c','b','m','p'):
+projection = AV_SPHERICAL_CUBEMAP;
+break;
+case MKTAG('e','q','u','i'):
+projection = AV_SPHERICAL_EQUIRECTANGULAR;
+break;
+default:
+av_log(c->fc, AV_LOG_ERROR, "Unknown projection type\n");
+
[FFmpeg-devel] [PATCH 1/4 v4] lavu: Add AVSphericalMapping type and frame side data
While no decoder currently exports spherical information, this type
represents a frame property that has to be passed through from container
to frames.
Signed-off-by: Vittorio Giovara
---
Revised documentation of the new type, removed bounding rectangle values,
as I misinterpreted what they actually did.
Vittorio
libavutil/Makefile| 2 +
libavutil/avutil.h| 6 +++
libavutil/frame.h | 8 ++-
libavutil/spherical.c | 34 +
libavutil/spherical.h | 133 ++
5 files changed, 182 insertions(+), 1 deletion(-)
create mode 100644 libavutil/spherical.c
create mode 100644 libavutil/spherical.h
diff --git a/libavutil/Makefile b/libavutil/Makefile
index c61ca9c..6474a30 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -63,6 +63,7 @@ HEADERS = adler32.h
\
samplefmt.h \
sha.h \
sha512.h \
+ spherical.h \
stereo3d.h\
threadmessage.h \
time.h\
@@ -140,6 +141,7 @@ OBJS = adler32.o
\
samplefmt.o \
sha.o\
sha512.o \
+ spherical.o \
stereo3d.o \
threadmessage.o \
time.o \
diff --git a/libavutil/avutil.h b/libavutil/avutil.h
index 29dd830..e9aaa03 100644
--- a/libavutil/avutil.h
+++ b/libavutil/avutil.h
@@ -118,6 +118,12 @@
*
* @}
*
+ * @defgroup lavu_video Video related
+ *
+ * @{
+ *
+ * @}
+ *
* @defgroup lavu_audio Audio related
*
* @{
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 8e51361..b450092 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -120,7 +120,13 @@ enum AVFrameSideDataType {
* The GOP timecode in 25 bit timecode format. Data format is 64-bit
integer.
* This is set on the first frame of a GOP that has a temporal reference
of 0.
*/
-AV_FRAME_DATA_GOP_TIMECODE
+AV_FRAME_DATA_GOP_TIMECODE,
+
+/**
+ * The data represents the AVSphericalMapping structure defined in
+ * libavutil/spherical.h.
+ */
+AV_FRAME_DATA_SPHERICAL,
};
enum AVActiveFormatDescription {
diff --git a/libavutil/spherical.c b/libavutil/spherical.c
new file mode 100644
index 000..816452c
--- /dev/null
+++ b/libavutil/spherical.c
@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2016 Vittorio Giovara
+ *
+ * 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 "mem.h"
+#include "spherical.h"
+
+AVSphericalMapping *av_spherical_alloc(size_t *size)
+{
+AVSphericalMapping *spherical = av_mallocz(sizeof(AVSphericalMapping));
+if (!spherical)
+return NULL;
+
+if (size)
+*size = sizeof(*spherical);
+
+return spherical;
+}
diff --git a/libavutil/spherical.h b/libavutil/spherical.h
new file mode 100644
index 000..9a61cb3
--- /dev/null
+++ b/libavutil/spherical.h
@@ -0,0 +1,133 @@
+/*
+ * Copyright (c) 2016 Vittorio Giovara
+ *
+ * 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 Lice
Re: [FFmpeg-devel] [PATCH 1/2] lavu: Add JEDEC P22 color primaries
On Wed, Nov 30, 2016 at 2:51 PM, Nicolas George wrote: > Le decadi 10 frimaire, an CCXXV, Vittorio Giovara a écrit : >> This is the value specified in the 23001-8_2013 document. > > This looks paywalled. Please give links to public versions of specs in > that kind of case. I am sorry, I don't have any public link I can share. > But I am pretty sure this document does not specify the values of enums > in FFmpeg's API. If the rest of the code requires that the values of the > enum match values in files or protocol, then I think a comment must say > so clearly. And if not, then the gap is not needed. It does. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavu: Add JEDEC P22 color primaries
On Wed, Nov 30, 2016 at 1:21 PM, Nicolas George wrote:
> Le decadi 10 frimaire, an CCXXV, Vittorio Giovara a écrit :
>> Signed-off-by: Vittorio Giovara
>> ---
>
>> Please CC.
>
> You can achieve the same result more reliably by setting the reply-to
> header.
Oh, nice trick, I'll try it out next time.
>> Vittorio
>>
>> libavutil/pixdesc.c | 1 +
>> libavutil/pixfmt.h | 1 +
>> 2 files changed, 2 insertions(+)
>>
>> diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
>> index 3b9c45d..04eab0b 100644
>> --- a/libavutil/pixdesc.c
>> +++ b/libavutil/pixdesc.c
>> @@ -2183,6 +2183,7 @@ static const char *color_primaries_names[AVCOL_PRI_NB]
>> = {
>> [AVCOL_PRI_SMPTE428] = "smpte428",
>> [AVCOL_PRI_SMPTE431] = "smpte431",
>> [AVCOL_PRI_SMPTE432] = "smpte432",
>> +[AVCOL_PRI_JEDEC_P22] = "jedec-p22",
>> };
>>
>> static const char *color_transfer_names[] = {
>> diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
>> index dfb1b11..d6c5a57 100644
>> --- a/libavutil/pixfmt.h
>> +++ b/libavutil/pixfmt.h
>> @@ -413,6 +413,7 @@ enum AVColorPrimaries {
>> AVCOL_PRI_SMPTEST428_1 = AVCOL_PRI_SMPTE428,
>> AVCOL_PRI_SMPTE431= 11, ///< SMPTE ST 431-2 (2011)
>> AVCOL_PRI_SMPTE432= 12, ///< SMPTE ST 432-1 D65 (2010)
>> +AVCOL_PRI_JEDEC_P22 = 22, ///< JEDEC P22 phosphors
>> AVCOL_PRI_NB///< Not part of ABI
>> };
>
> Are you leaving a gap on purpose? There was no gap until now.
This is the value specified in the 23001-8_2013 document.
> (Also, why are the values specified? And despite being "not part of the
> ABI", AVCOL_PRI_NB is used outside lavu.)
Hm those should probably be addressed too, the decoder and filters
ones are simple, while static options might be tricky.
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] vf_colorspace: Add support for jedec p22 primaries
Signed-off-by: Vittorio Giovara
---
(no I don't have samples, but the numbers should be correct)
Please CC.
Vittorio
doc/filters.texi| 3 +++
libavfilter/vf_colorspace.c | 2 ++
2 files changed, 5 insertions(+)
diff --git a/doc/filters.texi b/doc/filters.texi
index 4a4aeca..82d1047 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -5424,6 +5424,9 @@ SMPTE-432
@item bt2020
BT.2020
+@item jedec-p22
+JEDEC P22 phosphors
+
@end table
@anchor{range}
diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
index 0024505..8daa8f9 100644
--- a/libavfilter/vf_colorspace.c
+++ b/libavfilter/vf_colorspace.c
@@ -282,6 +282,7 @@ static const struct ColorPrimaries
color_primaries[AVCOL_PRI_NB] = {
[AVCOL_PRI_SMPTE432] = { WP_D65, 0.680, 0.320, 0.265, 0.690, 0.150, 0.060
},
[AVCOL_PRI_FILM] = { WP_C, 0.681, 0.319, 0.243, 0.692, 0.145, 0.049
},
[AVCOL_PRI_BT2020]= { WP_D65, 0.708, 0.292, 0.170, 0.797, 0.131, 0.046
},
+[AVCOL_PRI_JEDEC_P22] = { WP_D65, 0.630, 0.340, 0.290, 0.610, 0.160, 0.008
},
};
static const struct ColorPrimaries *get_color_primaries(enum AVColorPrimaries
prm)
@@ -1096,6 +1097,7 @@ static const AVOption colorspace_options[] = {
ENUM("smpte431", AVCOL_PRI_SMPTE431, "prm"),
ENUM("smpte432", AVCOL_PRI_SMPTE432, "prm"),
ENUM("bt2020", AVCOL_PRI_BT2020, "prm"),
+ENUM("jedec-p22",AVCOL_PRI_JEDEC_P22, "prm"),
{ "trc","Output transfer characteristics",
OFFSET(user_trc), AV_OPT_TYPE_INT, { .i64 = AVCOL_TRC_UNSPECIFIED },
--
2.10.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/2] lavu: Add JEDEC P22 color primaries
Signed-off-by: Vittorio Giovara
---
Please CC.
Vittorio
libavutil/pixdesc.c | 1 +
libavutil/pixfmt.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
index 3b9c45d..04eab0b 100644
--- a/libavutil/pixdesc.c
+++ b/libavutil/pixdesc.c
@@ -2183,6 +2183,7 @@ static const char *color_primaries_names[AVCOL_PRI_NB] = {
[AVCOL_PRI_SMPTE428] = "smpte428",
[AVCOL_PRI_SMPTE431] = "smpte431",
[AVCOL_PRI_SMPTE432] = "smpte432",
+[AVCOL_PRI_JEDEC_P22] = "jedec-p22",
};
static const char *color_transfer_names[] = {
diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
index dfb1b11..d6c5a57 100644
--- a/libavutil/pixfmt.h
+++ b/libavutil/pixfmt.h
@@ -413,6 +413,7 @@ enum AVColorPrimaries {
AVCOL_PRI_SMPTEST428_1 = AVCOL_PRI_SMPTE428,
AVCOL_PRI_SMPTE431= 11, ///< SMPTE ST 431-2 (2011)
AVCOL_PRI_SMPTE432= 12, ///< SMPTE ST 432-1 D65 (2010)
+AVCOL_PRI_JEDEC_P22 = 22, ///< JEDEC P22 phosphors
AVCOL_PRI_NB///< Not part of ABI
};
--
2.10.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] hevc: Support extradata changes
On Tue, Nov 29, 2016 at 7:20 AM, Michael Niedermayer wrote: > On Mon, Nov 28, 2016 at 10:03:37PM -0500, Vittorio Giovara wrote: >> On Mon, Nov 28, 2016 at 9:06 PM, Michael Niedermayer >> wrote: >> > On Tue, Nov 08, 2016 at 05:03:27PM -0500, Vittorio Giovara wrote: >> >> Signed-off-by: Vittorio Giovara >> >> --- >> >> Applied review. >> >> Please CC. >> >> Vittorio >> >> >> >> libavcodec/hevc.c | 10 ++ >> >> libavformat/mov.c | 4 >> > >> > please split this in 2 patches, the libavcodec one probably should >> > also have its version bumped as apps might want to depend on >> > a libavcodec with that feature >> >> ok for the version bumb, why splitting it 2 patches though? > > making changes to 2 libs at the same time can mask bugs because > you cannot checkout and test the intermediate but in distributions > users can end up with one lib updated and the other not (within what > the dependancies and versions allow) > So i always suggest spliting non cosmetic changes into a patch per lib > unless i miss/forget Well yes, but the changes in lavf only drop a log line, which is useless because of this patch, so imo it makes sense to keep them in the same diff. I'll split it if you insist though. Thanks -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] hevc: Support extradata changes
On Mon, Nov 28, 2016 at 9:06 PM, Michael Niedermayer wrote: > On Tue, Nov 08, 2016 at 05:03:27PM -0500, Vittorio Giovara wrote: >> Signed-off-by: Vittorio Giovara >> --- >> Applied review. >> Please CC. >> Vittorio >> >> libavcodec/hevc.c | 10 ++ >> libavformat/mov.c | 4 > > please split this in 2 patches, the libavcodec one probably should > also have its version bumped as apps might want to depend on > a libavcodec with that feature ok for the version bumb, why splitting it 2 patches though? -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] hevc: Allow parsing external extradata buffers
On Mon, Nov 28, 2016 at 9:07 PM, Michael Niedermayer
wrote:
> On Tue, Nov 29, 2016 at 03:01:28AM +0100, Michael Niedermayer wrote:
>> On Tue, Nov 08, 2016 at 05:03:26PM -0500, Vittorio Giovara wrote:
>> > ---
>> > As mentioned in the discussion.
>> > Please CC.
>> > Vittorio
>> >
>> > libavcodec/hevc.c | 12 +---
>> > 1 file changed, 5 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/libavcodec/hevc.c b/libavcodec/hevc.c
>> > index 29e0d49..02fd606 100644
>> > --- a/libavcodec/hevc.c
>> > +++ b/libavcodec/hevc.c
>> > @@ -2973,17 +2973,15 @@ static int verify_md5(HEVCContext *s, AVFrame
>> > *frame)
>> > return 0;
>> > }
>> >
>> > -static int hevc_decode_extradata(HEVCContext *s)
>> > +static int hevc_decode_extradata(HEVCContext *s, uint8_t *buf, int length)
>> > {
>> > AVCodecContext *avctx = s->avctx;
>> > GetByteContext gb;
>> > int ret, i;
>> >
>> > -bytestream2_init(&gb, avctx->extradata, avctx->extradata_size);
>> > +bytestream2_init(&gb, buf, length);
>> >
>> > -if (avctx->extradata_size > 3 &&
>> > -(avctx->extradata[0] || avctx->extradata[1] ||
>> > - avctx->extradata[2] > 1)) {
>> > +if (avctx->extradata_size > 3 && (buf[0] || buf[1] || buf[2] > 1)) {
>>^
>>
>> is that intended to stay extradata_size ?
ops, no, good catch
applied
>> > +if (length > 3 && (buf[0] || buf[1] || buf[2] > 1)) {
locally, thanks
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] hevc: Support extradata changes
On Thu, Nov 17, 2016 at 10:38 AM, Vittorio Giovara wrote: > On Tue, Nov 8, 2016 at 5:03 PM, Vittorio Giovara > wrote: >> Signed-off-by: Vittorio Giovara >> --- Hi, if no further objections I'll push the set tomorrow. o hevc: Support extradata changes o hevc: Allow parsing external extradata buffers Please CC. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] mov: Evaluate the movie display matrix
On Fri, Nov 18, 2016 at 2:37 PM, Vittorio Giovara wrote: > This matrix needs to be applied after all others have (currently only > display matrix from trak), but cannot be handled in movie box, since > streams are not allocated yet. So store it in main context, and apply > it when appropriate, that is after parsing the tkhd one. > > Signed-off-by: Vittorio Giovara > --- Hi, if no further objections I'll push the set tomorrow. o fate: Add test for mov displaymatrix o ffprobe: Fix displaying side data list only o mov: Evaluate the movie display matrix Please CC me. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/3] fate: Add test for mov displaymatrix
Signed-off-by: Vittorio Giovara --- Updated according James' review. Vittorio tests/fate/mov.mak | 6 +- tests/ref/fate/mov-displaymatrix | 12 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 tests/ref/fate/mov-displaymatrix diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak index bb02d6b..71c9a1f 100644 --- a/tests/fate/mov.mak +++ b/tests/fate/mov.mak @@ -7,7 +7,8 @@ FATE_MOV = fate-mov-3elist \ fate-mov-2elist-elist1-ends-bframe \ fate-mov-zombie \ fate-mov-aac-2048-priming \ - fate-mp4-init-nonkeyframe + fate-mp4-init-nonkeyframe \ + fate-mov-displaymatrix \ FATE_SAMPLES_AVCONV += $(FATE_MOV) @@ -38,3 +39,6 @@ fate-mov-zombie: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_streams -show_packe fate-mp4-init-nonkeyframe: ffprobe$(PROGSSUF)$(EXESUF) fate-mp4-init-nonkeyframe: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_packets -print_format compact -select_streams v $(TARGET_SAMPLES)/mov/mp4-init-nonkeyframe.mp4 + +fate-mp4-displaymatrix: ffprobe$(PROGSSUF)$(EXESUF) +fate-mov-displaymatrix: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_entries stream_side_data_list -select_streams v -v 0 $(TARGET_SAMPLES)/mov/displaymatrix.mov diff --git a/tests/ref/fate/mov-displaymatrix b/tests/ref/fate/mov-displaymatrix new file mode 100644 index 000..86139fc --- /dev/null +++ b/tests/ref/fate/mov-displaymatrix @@ -0,0 +1,12 @@ +[STREAM] +[SIDE_DATA] +side_data_type=Display Matrix +side_data_size=36 +displaymatrix= +:0 131072 0 +0001: -65536 0 0 +0002: 47185920 0 1073741824 + +rotation=-90 +[/SIDE_DATA] +[/STREAM] -- 2.10.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/3] mov: Evaluate the movie display matrix
This matrix needs to be applied after all others have (currently only
display matrix from trak), but cannot be handled in movie box, since
streams are not allocated yet. So store it in main context, and apply
it when appropriate, that is after parsing the tkhd one.
Signed-off-by: Vittorio Giovara
---
No changes, except I moved tests to a separate commit.
Please CC.
Vittorio
libavformat/isom.h | 1 +
libavformat/mov.c | 48 +++-
2 files changed, 36 insertions(+), 13 deletions(-)
diff --git a/libavformat/isom.h b/libavformat/isom.h
index d684502..02bfedd 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -240,6 +240,7 @@ typedef struct MOVContext {
uint8_t *decryption_key;
int decryption_key_len;
int enable_drefs;
+int32_t movie_display_matrix[3][3]; ///< display matrix from mvhd
} MOVContext;
int ff_mp4_read_descr_len(AVIOContext *pb);
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 8d6cc12..b7d0b12 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1239,6 +1239,7 @@ static int mov_read_mdhd(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
static int mov_read_mvhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
{
+int i;
int64_t creation_time;
int version = avio_r8(pb); /* version */
avio_rb24(pb); /* flags */
@@ -1269,7 +1270,12 @@ static int mov_read_mvhd(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
avio_skip(pb, 10); /* reserved */
-avio_skip(pb, 36); /* display matrix */
+/* movie display matrix, store it in main context and use it later on */
+for (i = 0; i < 3; i++) {
+c->movie_display_matrix[i][0] = avio_rb32(pb); // 16.16 fixed point
+c->movie_display_matrix[i][1] = avio_rb32(pb); // 16.16 fixed point
+c->movie_display_matrix[i][2] = avio_rb32(pb); // 2.30 fixed point
+}
avio_rb32(pb); /* preview time */
avio_rb32(pb); /* preview duration */
@@ -3847,12 +3853,22 @@ static int mov_read_meta(MOVContext *c, AVIOContext
*pb, MOVAtom atom)
return 0;
}
+// return 1 when matrix is identity, 0 otherwise
+#define IS_MATRIX_IDENT(matrix)\
+( (matrix)[0][0] == (1 << 16) && \
+ (matrix)[1][1] == (1 << 16) && \
+ (matrix)[2][2] == (1 << 30) && \
+ !(matrix)[0][1] && !(matrix)[0][2] && \
+ !(matrix)[1][0] && !(matrix)[1][2] && \
+ !(matrix)[2][0] && !(matrix)[2][1])
+
static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
{
-int i;
+int i, j, e;
int width;
int height;
int display_matrix[3][3];
+int res_display_matrix[3][3] = { { 0 } };
AVStream *st;
MOVStreamContext *sc;
int version;
@@ -3902,15 +3918,20 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext
*pb, MOVAtom atom)
sc->width = width >> 16;
sc->height = height >> 16;
-// save the matrix and add rotate metadata when it is not the default
-// identity
-if (display_matrix[0][0] != (1 << 16) ||
-display_matrix[1][1] != (1 << 16) ||
-display_matrix[2][2] != (1 << 30) ||
-display_matrix[0][1] || display_matrix[0][2] ||
-display_matrix[1][0] || display_matrix[1][2] ||
-display_matrix[2][0] || display_matrix[2][1]) {
-int i, j;
+// apply the moov display matrix (after the tkhd one)
+for (i = 0; i < 3; i++) {
+const int sh[3] = { 16, 16, 30 };
+for (j = 0; j < 3; j++) {
+for (e = 0; e < 3; e++) {
+res_display_matrix[i][j] +=
+((int64_t) display_matrix[i][e] *
+ c->movie_display_matrix[e][j]) >> sh[e];
+}
+}
+}
+
+// save the matrix when it is not the default identity
+if (!IS_MATRIX_IDENT(res_display_matrix)) {
double rotate;
av_freep(&sc->display_matrix);
@@ -3920,7 +3941,7 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
for (i = 0; i < 3; i++)
for (j = 0; j < 3; j++)
-sc->display_matrix[i * 3 + j] = display_matrix[i][j];
+sc->display_matrix[i * 3 + j] = res_display_matrix[i][j];
rotate = av_display_rotation_get(sc->display_matrix);
if (!isnan(rotate)) {
@@ -3939,7 +3960,8 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
double disp_transform[2];
for (i = 0; i < 2; i++)
-disp_transform[i] = hypot(display_matrix[i][0],
display_matrix[i][1]);
+disp_transform[i] = hypot(sc->display_matrix[0 + i],
+ sc->display_matrix[3 + i]);
if (disp_transform[0] > 0 && disp_transform[1] > 0 &&
disp_transform[0] < (1<<24) && disp_transform[1] < (1<<24) &&
--
2.10.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/3] ffprobe: Fix displaying side data list only
Signed-off-by: Vittorio Giovara
---
Needed by the following commit.
Please CC.
Vittorio
ffprobe.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/ffprobe.c b/ffprobe.c
index 79fe296..4ed8e06 100644
--- a/ffprobe.c
+++ b/ffprobe.c
@@ -188,7 +188,7 @@ static struct section sections[] = {
[SECTION_ID_FRAMES] = { SECTION_ID_FRAMES, "frames",
SECTION_FLAG_IS_ARRAY, { SECTION_ID_FRAME, SECTION_ID_SUBTITLE, -1 } },
[SECTION_ID_FRAME] = { SECTION_ID_FRAME, "frame", 0, {
SECTION_ID_FRAME_TAGS, SECTION_ID_FRAME_SIDE_DATA_LIST, -1 } },
[SECTION_ID_FRAME_TAGS] = { SECTION_ID_FRAME_TAGS, "tags",
SECTION_FLAG_HAS_VARIABLE_FIELDS, { -1 }, .element_name = "tag", .unique_name =
"frame_tags" },
-[SECTION_ID_FRAME_SIDE_DATA_LIST] ={ SECTION_ID_FRAME_SIDE_DATA_LIST,
"side_data_list", SECTION_FLAG_IS_ARRAY, { SECTION_ID_FRAME_SIDE_DATA, -1 } },
+[SECTION_ID_FRAME_SIDE_DATA_LIST] ={ SECTION_ID_FRAME_SIDE_DATA_LIST,
"side_data_list", SECTION_FLAG_IS_ARRAY, { SECTION_ID_FRAME_SIDE_DATA, -1 },
.element_name = "side_data", .unique_name = "frame_side_data_list" },
[SECTION_ID_FRAME_SIDE_DATA] = { SECTION_ID_FRAME_SIDE_DATA,
"side_data", 0, { -1 } },
[SECTION_ID_LIBRARY_VERSIONS] = { SECTION_ID_LIBRARY_VERSIONS,
"library_versions", SECTION_FLAG_IS_ARRAY, { SECTION_ID_LIBRARY_VERSION, -1 } },
[SECTION_ID_LIBRARY_VERSION] ={ SECTION_ID_LIBRARY_VERSION,
"library_version", 0, { -1 } },
@@ -196,7 +196,7 @@ static struct section sections[] = {
[SECTION_ID_PACKETS_AND_FRAMES] = { SECTION_ID_PACKETS_AND_FRAMES,
"packets_and_frames", SECTION_FLAG_IS_ARRAY, { SECTION_ID_PACKET, -1} },
[SECTION_ID_PACKET] = { SECTION_ID_PACKET, "packet", 0, {
SECTION_ID_PACKET_TAGS, SECTION_ID_PACKET_SIDE_DATA_LIST, -1 } },
[SECTION_ID_PACKET_TAGS] ={ SECTION_ID_PACKET_TAGS, "tags",
SECTION_FLAG_HAS_VARIABLE_FIELDS, { -1 }, .element_name = "tag", .unique_name =
"packet_tags" },
-[SECTION_ID_PACKET_SIDE_DATA_LIST] ={ SECTION_ID_PACKET_SIDE_DATA_LIST,
"side_data_list", SECTION_FLAG_IS_ARRAY, { SECTION_ID_PACKET_SIDE_DATA, -1 } },
+[SECTION_ID_PACKET_SIDE_DATA_LIST] ={ SECTION_ID_PACKET_SIDE_DATA_LIST,
"side_data_list", SECTION_FLAG_IS_ARRAY, { SECTION_ID_PACKET_SIDE_DATA, -1 },
.element_name = "side_data", .unique_name = "packet_side_data_list" },
[SECTION_ID_PACKET_SIDE_DATA] = { SECTION_ID_PACKET_SIDE_DATA,
"side_data", 0, { -1 } },
[SECTION_ID_PIXEL_FORMATS] = { SECTION_ID_PIXEL_FORMATS,
"pixel_formats", SECTION_FLAG_IS_ARRAY, { SECTION_ID_PIXEL_FORMAT, -1 } },
[SECTION_ID_PIXEL_FORMAT] = { SECTION_ID_PIXEL_FORMAT,
"pixel_format", 0, { SECTION_ID_PIXEL_FORMAT_FLAGS,
SECTION_ID_PIXEL_FORMAT_COMPONENTS, -1 } },
@@ -219,7 +219,7 @@ static struct section sections[] = {
[SECTION_ID_STREAM] = { SECTION_ID_STREAM, "stream", 0, {
SECTION_ID_STREAM_DISPOSITION, SECTION_ID_STREAM_TAGS,
SECTION_ID_STREAM_SIDE_DATA_LIST, -1 } },
[SECTION_ID_STREAM_DISPOSITION] = { SECTION_ID_STREAM_DISPOSITION,
"disposition", 0, { -1 }, .unique_name = "stream_disposition" },
[SECTION_ID_STREAM_TAGS] ={ SECTION_ID_STREAM_TAGS, "tags",
SECTION_FLAG_HAS_VARIABLE_FIELDS, { -1 }, .element_name = "tag", .unique_name =
"stream_tags" },
-[SECTION_ID_STREAM_SIDE_DATA_LIST] ={ SECTION_ID_STREAM_SIDE_DATA_LIST,
"side_data_list", SECTION_FLAG_IS_ARRAY, { SECTION_ID_STREAM_SIDE_DATA, -1 } },
+[SECTION_ID_STREAM_SIDE_DATA_LIST] ={ SECTION_ID_STREAM_SIDE_DATA_LIST,
"side_data_list", SECTION_FLAG_IS_ARRAY, { SECTION_ID_STREAM_SIDE_DATA, -1 },
.element_name = "side_data", .unique_name = "stream_side_data_list" },
[SECTION_ID_STREAM_SIDE_DATA] = { SECTION_ID_STREAM_SIDE_DATA,
"side_data", 0, { -1 } },
[SECTION_ID_SUBTITLE] = { SECTION_ID_SUBTITLE, "subtitle", 0, {
-1 } },
};
--
2.10.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv2] vf_colorspace: Forbid odd dimensions
On Thu, Nov 17, 2016 at 4:11 PM, Ronald S. Bultje wrote:
> Hi,
>
> On Thu, Nov 17, 2016 at 4:09 PM, Vittorio Giovara
> wrote:
>>
>> This prevents writing past bounds.
>>
>> Signed-off-by: Vittorio Giovara
>> ---
>> Updated according review.
>> Vittorio
>>
>> doc/filters.texi| 1 +
>> libavfilter/vf_colorspace.c | 9 -
>> 2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/doc/filters.texi b/doc/filters.texi
>> index 208be42..aee43a8 100644
>> --- a/doc/filters.texi
>> +++ b/doc/filters.texi
>> @@ -5278,6 +5278,7 @@ colormatrix=bt601:smpte240m
>> @section colorspace
>>
>> Convert colorspace, transfer characteristics or color primaries.
>> +Input video needs to have an even size.
>>
>> The filter accepts the following options:
>>
>> diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
>> index c7a2286..0024505 100644
>> --- a/libavfilter/vf_colorspace.c
>> +++ b/libavfilter/vf_colorspace.c
>> @@ -168,7 +168,7 @@ typedef struct ColorSpaceContext {
>> int did_warn_range;
>> } ColorSpaceContext;
>>
>> -// FIXME deal with odd width/heights (or just forbid it)
>> +// FIXME deal with odd width/heights
>> // FIXME faster linearize/delinearize implementation (integer pow)
>> // FIXME bt2020cl support (linearization between yuv/rgb step instead of
>> between rgb/xyz)
>> // FIXME test that the values in (de)lin_lut don't exceed their container
>> storage
>> @@ -1031,8 +1031,15 @@ static int query_formats(AVFilterContext *ctx)
>>
>> static int config_props(AVFilterLink *outlink)
>> {
>> +AVFilterContext *ctx = outlink->dst;
>> AVFilterLink *inlink = outlink->src->inputs[0];
>>
>> +if (inlink->w % 2 || inlink->h % 2) {
>> +av_log(ctx, AV_LOG_ERROR, "Invalid odd size (%dx%d)\n",
>> + inlink->w, inlink->h);
>> +return AVERROR_PATCHWELCOME;
>> +}
>
>
> If 422 && w%2 or 420 && (w%2||h%2), or some variant thereof. Odd sizes for
> 444 or odd vertical sizes for 422 should be OK, right?
well in theory, but i actually got a segfault on 422
==4064== Invalid write of size 2
==4064==at 0x4ED6637: yuv2rgb_422p10_c (colorspacedsp_template.c:89)
==4064==by 0x4F0444C: convert (vf_colorspace.c:504)
and on 444 (with testsrc).
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCHv2] vf_colorspace: Forbid odd dimensions
This prevents writing past bounds.
Signed-off-by: Vittorio Giovara
---
Updated according review.
Vittorio
doc/filters.texi| 1 +
libavfilter/vf_colorspace.c | 9 -
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index 208be42..aee43a8 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -5278,6 +5278,7 @@ colormatrix=bt601:smpte240m
@section colorspace
Convert colorspace, transfer characteristics or color primaries.
+Input video needs to have an even size.
The filter accepts the following options:
diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
index c7a2286..0024505 100644
--- a/libavfilter/vf_colorspace.c
+++ b/libavfilter/vf_colorspace.c
@@ -168,7 +168,7 @@ typedef struct ColorSpaceContext {
int did_warn_range;
} ColorSpaceContext;
-// FIXME deal with odd width/heights (or just forbid it)
+// FIXME deal with odd width/heights
// FIXME faster linearize/delinearize implementation (integer pow)
// FIXME bt2020cl support (linearization between yuv/rgb step instead of
between rgb/xyz)
// FIXME test that the values in (de)lin_lut don't exceed their container
storage
@@ -1031,8 +1031,15 @@ static int query_formats(AVFilterContext *ctx)
static int config_props(AVFilterLink *outlink)
{
+AVFilterContext *ctx = outlink->dst;
AVFilterLink *inlink = outlink->src->inputs[0];
+if (inlink->w % 2 || inlink->h % 2) {
+av_log(ctx, AV_LOG_ERROR, "Invalid odd size (%dx%d)\n",
+ inlink->w, inlink->h);
+return AVERROR_PATCHWELCOME;
+}
+
outlink->w = inlink->w;
outlink->h = inlink->h;
outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
--
2.10.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] vf_colorspace: Forbid odd dimensions
This prevents writing past bounds.
Signed-off-by: Vittorio Giovara
---
Any opinions about this?
Cheers,
Vittorio
(please cc)
doc/filters.texi| 1 +
libavfilter/vf_colorspace.c | 8 +++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index 208be42..aee43a8 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -5278,6 +5278,7 @@ colormatrix=bt601:smpte240m
@section colorspace
Convert colorspace, transfer characteristics or color primaries.
+Input video needs to have an even size.
The filter accepts the following options:
diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
index c7a2286..5bc7b3c 100644
--- a/libavfilter/vf_colorspace.c
+++ b/libavfilter/vf_colorspace.c
@@ -168,7 +168,6 @@ typedef struct ColorSpaceContext {
int did_warn_range;
} ColorSpaceContext;
-// FIXME deal with odd width/heights (or just forbid it)
// FIXME faster linearize/delinearize implementation (integer pow)
// FIXME bt2020cl support (linearization between yuv/rgb step instead of
between rgb/xyz)
// FIXME test that the values in (de)lin_lut don't exceed their container
storage
@@ -1031,8 +1030,15 @@ static int query_formats(AVFilterContext *ctx)
static int config_props(AVFilterLink *outlink)
{
+AVFilterContext *ctx = outlink->dst;
AVFilterLink *inlink = outlink->src->inputs[0];
+if (inlink->w % 2 || inlink->h % 2) {
+av_log(ctx, AV_LOG_ERROR, "Invalid odd size (%dx%d)\n",
+ inlink->w, inlink->h);
+return AVERROR(EINVAL);
+}
+
outlink->w = inlink->w;
outlink->h = inlink->h;
outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
--
2.10.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] vf_colorspace: Forbid odd dimensions
On Thu, Nov 17, 2016 at 3:58 PM, Ronald S. Bultje wrote: > Hi, > > On Thu, Nov 17, 2016 at 3:53 PM, Vittorio Giovara > wrote: >> >> This prevents writing past bounds. >> >> Signed-off-by: Vittorio Giovara >> --- >> Any opinions about this? > > > Hm... I agree right now it's probably broken, but I'd argue that we should > fix it, not forbid it. > > That may be out of scope for you, but then at least don't remove the FIXME > in the TODO list, and output AVERROR_PATCHWELCOME? sure, as you prefer > But maybe I'm thinking about this too much/idealistically... :) -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv5] mov: Evaluate the movie display matrix
On Thu, Nov 17, 2016 at 1:02 PM, James Almer wrote: > On 11/15/2016 1:14 PM, Vittorio Giovara wrote: >> diff --git a/tests/ref/fate/mov-displaymatrix >> b/tests/ref/fate/mov-displaymatrix >> new file mode 100644 >> index 000..52528c1 >> --- /dev/null >> +++ b/tests/ref/fate/mov-displaymatrix >> @@ -0,0 +1,10 @@ >> +#format: frame checksums >> +#version: 2 >> +#hash: MD5 >> +#tb 0: 1001/3 >> +#media_type 0: video >> +#codec_id 0: rawvideo >> +#dimensions 0: 240x160 >> +#sar 0: 2/1 >> +#stream#, dts,pts, duration, size, hash >> +0, 0, 0,1,57600, >> be949aa661551010f461069804f68e76 >> > > The display matrix doesn't seem to affect decoding with ffmpeg so this > isn't really testing anything other than h264 decoding. The `sar` field is affected, but yeah it's probably better to use ffprobe -show_streams. Thanks, -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH v2] mov: Export spherical information
This implements Spherical Video V1 and V2, as described in the
spatial-media collection by Google.
Signed-off-by: Vittorio Giovara
---
Updated following James' review.
Please CC.
Vittorio
libavformat/isom.h | 7 ++
libavformat/mov.c | 309 -
2 files changed, 315 insertions(+), 1 deletion(-)
diff --git a/libavformat/isom.h b/libavformat/isom.h
index 02bfedd..0fd9eb0 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -24,6 +24,9 @@
#ifndef AVFORMAT_ISOM_H
#define AVFORMAT_ISOM_H
+#include "libavutil/spherical.h"
+#include "libavutil/stereo3d.h"
+
#include "avio.h"
#include "internal.h"
#include "dv.h"
@@ -177,6 +180,10 @@ typedef struct MOVStreamContext {
int stsd_count;
int32_t *display_matrix;
+AVStereo3D *stereo3d;
+AVSphericalMapping *spherical;
+size_t spherical_size;
+
uint32_t format;
int has_sidx; // If there is an sidx entry for this stream.
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 6beb027..7d02fe0 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -42,6 +42,8 @@
#include "libavutil/aes.h"
#include "libavutil/aes_ctr.h"
#include "libavutil/sha.h"
+#include "libavutil/spherical.h"
+#include "libavutil/stereo3d.h"
#include "libavutil/timecode.h"
#include "libavcodec/ac3tab.h"
#include "libavcodec/mpegaudiodecheader.h"
@@ -4497,8 +4499,258 @@ static int mov_read_tmcd(MOVContext *c, AVIOContext
*pb, MOVAtom atom)
return 0;
}
+static int mov_read_st3d(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+AVStream *st;
+MOVStreamContext *sc;
+enum AVStereo3DType type;
+int mode;
+
+if (c->fc->nb_streams < 1)
+return 0;
+
+st = c->fc->streams[c->fc->nb_streams - 1];
+sc = st->priv_data;
+
+if (atom.size < 5) {
+av_log(c->fc, AV_LOG_ERROR, "Empty stereoscopic video box\n");
+return AVERROR_INVALIDDATA;
+}
+
+mode = avio_r8(pb);
+switch (mode) {
+case 0:
+type = AV_STEREO3D_2D;
+break;
+case 1:
+type = AV_STEREO3D_TOPBOTTOM;
+break;
+case 2:
+type = AV_STEREO3D_SIDEBYSIDE;
+break;
+default:
+av_log(c->fc, AV_LOG_WARNING, "Unknown st3d mode value %d\n", mode);
+return 0;
+}
+
+sc->stereo3d = av_stereo3d_alloc();
+if (!sc->stereo3d)
+return AVERROR(ENOMEM);
+
+sc->stereo3d->type = type;
+return 0;
+}
+
+static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+AVStream *st;
+MOVStreamContext *sc;
+int version, size;
+int32_t yaw, pitch, roll;
+uint32_t tag;
+unsigned l, t, r, b;
+enum AVSphericalProjection projection;
+
+if (c->fc->nb_streams < 1)
+return 0;
+
+st = c->fc->streams[c->fc->nb_streams - 1];
+sc = st->priv_data;
+
+if (atom.size < 8) {
+av_log(c->fc, AV_LOG_ERROR, "Empty spherical video box\n");
+return AVERROR_INVALIDDATA;
+}
+
+version = avio_r8(pb);
+if (version) {
+av_log(c->fc, AV_LOG_WARNING, "Unsupported svhd box version (%d)\n",
+ version);
+return 0;
+}
+
+size = avio_rb24(pb);
+if (size > atom.size)
+return AVERROR_INVALIDDATA;
+
+tag = avio_rl32(pb);
+if (tag != MKTAG('s','v','h','d')) {
+av_log(c->fc, AV_LOG_ERROR, "Missing spherical video header\n");
+return 0;
+}
+avio_skip(pb, size - 8); /* metadata_source */
+
+version = avio_r8(pb);
+if (version) {
+av_log(c->fc, AV_LOG_WARNING, "Unsupported proj box version (%d)\n",
+ version);
+return 0;
+}
+
+size = avio_rb24(pb);
+if (size > atom.size)
+return AVERROR_INVALIDDATA;
+
+tag = avio_rl32(pb);
+if (tag != MKTAG('p','r','o','j')) {
+av_log(c->fc, AV_LOG_ERROR, "Missing projection box\n");
+return 0;
+}
+
+version = avio_r8(pb);
+if (version) {
+av_log(c->fc, AV_LOG_WARNING, "Unsupported prhd box version (%d)\n",
+ version);
+return 0;
+}
+
+size = avio_rb24(pb);
+if (size > atom.size)
+return AVERROR_INVALIDDATA;
+
+tag = avio_rl32(pb);
+if (tag != MKTAG('p','r','h','d')) {
+av_log(c->fc, AV_LOG_ERROR, "Missing projection header box\n");
+return 0;
+}
+
+/* 16.16 fixed point */
+yaw = avio_rb32(pb);
+pitch = avio_rb32(pb);
+roll = avio_rb32(pb);
+
+avio_skip(pb, size - 20);
+
+version = avio_r8
Re: [FFmpeg-devel] [PATCHv5] mov: Evaluate the movie display matrix
On Tue, Nov 15, 2016 at 11:14 AM, Vittorio Giovara
wrote:
> This matrix needs to be applied after all others have (currently only
> display matrix from trak), but cannot be handled in movie box, since
> streams are not allocated yet. So store it in main context, and apply
> it when appropriate, that is after parsing the tkhd one.
>
> Signed-off-by: Vittorio Giovara
> ---
> Fixed a boolean operation. No other changes, ready to be committed.
> Please CC.
> Vittorio
>
> libavformat/isom.h | 1 +
> libavformat/mov.c| 48
> +---
> tests/fate/mov.mak | 5 -
> tests/ref/fate/mov-displaymatrix | 10 +
> 4 files changed, 50 insertions(+), 14 deletions(-)
> create mode 100644 tests/ref/fate/mov-displaymatrix
>
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index d684502..02bfedd 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -240,6 +240,7 @@ typedef struct MOVContext {
> uint8_t *decryption_key;
> int decryption_key_len;
> int enable_drefs;
> +int32_t movie_display_matrix[3][3]; ///< display matrix from mvhd
> } MOVContext;
>
> int ff_mp4_read_descr_len(AVIOContext *pb);
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 8d6cc12..b7d0b12 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -1239,6 +1239,7 @@ static int mov_read_mdhd(MOVContext *c, AVIOContext
> *pb, MOVAtom atom)
>
> static int mov_read_mvhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> {
> +int i;
> int64_t creation_time;
> int version = avio_r8(pb); /* version */
> avio_rb24(pb); /* flags */
> @@ -1269,7 +1270,12 @@ static int mov_read_mvhd(MOVContext *c, AVIOContext
> *pb, MOVAtom atom)
>
> avio_skip(pb, 10); /* reserved */
>
> -avio_skip(pb, 36); /* display matrix */
> +/* movie display matrix, store it in main context and use it later on */
> +for (i = 0; i < 3; i++) {
> +c->movie_display_matrix[i][0] = avio_rb32(pb); // 16.16 fixed point
> +c->movie_display_matrix[i][1] = avio_rb32(pb); // 16.16 fixed point
> +c->movie_display_matrix[i][2] = avio_rb32(pb); // 2.30 fixed point
> +}
>
> avio_rb32(pb); /* preview time */
> avio_rb32(pb); /* preview duration */
> @@ -3847,12 +3853,22 @@ static int mov_read_meta(MOVContext *c, AVIOContext
> *pb, MOVAtom atom)
> return 0;
> }
>
> +// return 1 when matrix is identity, 0 otherwise
> +#define IS_MATRIX_IDENT(matrix)\
> +( (matrix)[0][0] == (1 << 16) && \
> + (matrix)[1][1] == (1 << 16) && \
> + (matrix)[2][2] == (1 << 30) && \
> + !(matrix)[0][1] && !(matrix)[0][2] && \
> + !(matrix)[1][0] && !(matrix)[1][2] && \
> + !(matrix)[2][0] && !(matrix)[2][1])
> +
> static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> {
> -int i;
> +int i, j, e;
> int width;
> int height;
> int display_matrix[3][3];
> +int res_display_matrix[3][3] = { { 0 } };
> AVStream *st;
> MOVStreamContext *sc;
> int version;
> @@ -3902,15 +3918,20 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext
> *pb, MOVAtom atom)
> sc->width = width >> 16;
> sc->height = height >> 16;
>
> -// save the matrix and add rotate metadata when it is not the default
> -// identity
> -if (display_matrix[0][0] != (1 << 16) ||
> -display_matrix[1][1] != (1 << 16) ||
> -display_matrix[2][2] != (1 << 30) ||
> -display_matrix[0][1] || display_matrix[0][2] ||
> -display_matrix[1][0] || display_matrix[1][2] ||
> -display_matrix[2][0] || display_matrix[2][1]) {
> -int i, j;
> +// apply the moov display matrix (after the tkhd one)
> +for (i = 0; i < 3; i++) {
> +const int sh[3] = { 16, 16, 30 };
> +for (j = 0; j < 3; j++) {
> +for (e = 0; e < 3; e++) {
> +res_display_matrix[i][j] +=
> +((int64_t) display_matrix[i][e] *
> + c->movie_display_matrix[e][j]) >> sh[e];
> +}
> +}
> +}
> +
> +// save the matrix when it is not the default identity
> +if (!IS_MATRIX_IDENT(res_display_matrix)) {
> double rotate;
>
> av_freep(&sc->display_matrix);
> @@ -3920,7 +3941,7 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext
> *pb, MOVAtom atom)
>
> for (i = 0; i < 3; i++)
>
Re: [FFmpeg-devel] [PATCH 1/3] hevc: Allow parsing external extradata buffers
On Tue, Nov 8, 2016 at 5:03 PM, Vittorio Giovara
wrote:
> ---
> As mentioned in the discussion.
> Please CC.
> Vittorio
>
> libavcodec/hevc.c | 12 +---
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/libavcodec/hevc.c b/libavcodec/hevc.c
> index 29e0d49..02fd606 100644
> --- a/libavcodec/hevc.c
> +++ b/libavcodec/hevc.c
> @@ -2973,17 +2973,15 @@ static int verify_md5(HEVCContext *s, AVFrame *frame)
> return 0;
> }
>
> -static int hevc_decode_extradata(HEVCContext *s)
> +static int hevc_decode_extradata(HEVCContext *s, uint8_t *buf, int length)
> {
> AVCodecContext *avctx = s->avctx;
> GetByteContext gb;
> int ret, i;
>
> -bytestream2_init(&gb, avctx->extradata, avctx->extradata_size);
> +bytestream2_init(&gb, buf, length);
>
> -if (avctx->extradata_size > 3 &&
> -(avctx->extradata[0] || avctx->extradata[1] ||
> - avctx->extradata[2] > 1)) {
> +if (avctx->extradata_size > 3 && (buf[0] || buf[1] || buf[2] > 1)) {
> /* It seems the extradata is encoded as hvcC format.
> * Temporarily, we support configurationVersion==0 until 14496-15 3rd
> * is finalized. When finalized, configurationVersion will be 1 and
> we
> @@ -3030,7 +3028,7 @@ static int hevc_decode_extradata(HEVCContext *s)
> s->nal_length_size = nal_len_size;
> } else {
> s->is_nalff = 0;
> -ret = decode_nal_units(s, avctx->extradata, avctx->extradata_size);
> +ret = decode_nal_units(s, buf, length);
> if (ret < 0)
> return ret;
> }
> @@ -3338,7 +3336,7 @@ static av_cold int hevc_decode_init(AVCodecContext
> *avctx)
> s->threads_number = 1;
>
> if (avctx->extradata_size > 0 && avctx->extradata) {
> -ret = hevc_decode_extradata(s);
> +ret = hevc_decode_extradata(s, avctx->extradata,
> avctx->extradata_size);
> if (ret < 0) {
> hevc_decode_free(avctx);
> return ret;
> --
> 2.10.0
>
ping
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] hevc: Support extradata changes
On Tue, Nov 8, 2016 at 5:03 PM, Vittorio Giovara
wrote:
> Signed-off-by: Vittorio Giovara
> ---
> Applied review.
> Please CC.
> Vittorio
>
> libavcodec/hevc.c | 10 ++
> libavformat/mov.c | 4
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/hevc.c b/libavcodec/hevc.c
> index 02fd606..4417f79 100644
> --- a/libavcodec/hevc.c
> +++ b/libavcodec/hevc.c
> @@ -3049,6 +3049,8 @@ static int hevc_decode_frame(AVCodecContext *avctx,
> void *data, int *got_output,
> AVPacket *avpkt)
> {
> int ret;
> +int new_extradata_size;
> +uint8_t *new_extradata;
> HEVCContext *s = avctx->priv_data;
>
> if (!avpkt->size) {
> @@ -3060,6 +3062,14 @@ static int hevc_decode_frame(AVCodecContext *avctx,
> void *data, int *got_output,
> return 0;
> }
>
> +new_extradata = av_packet_get_side_data(avpkt, AV_PKT_DATA_NEW_EXTRADATA,
> +&new_extradata_size);
> +if (new_extradata && new_extradata_size > 0) {
> +ret = hevc_decode_extradata(s, new_extradata, new_extradata_size);
> +if (ret < 0)
> +return ret;
> +}
> +
> s->ref = NULL;
> ret= decode_nal_units(s, avpkt->data, avpkt->size);
> if (ret < 0)
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 2fc09b1..a2a688b 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -2231,10 +2231,6 @@ static int mov_skip_multiple_stsd(MOVContext *c,
> AVIOContext *pb,
> avio_skip(pb, size);
> return 1;
> }
> -if ( codec_tag == AV_RL32("hvc1") ||
> - codec_tag == AV_RL32("hev1")
> -)
> -av_log(c->fc, AV_LOG_WARNING, "Concatenated H.264 or H.265 might not
> play correctly.\n");
>
> return 0;
> }
> --
> 2.10.0
>
ping
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3 v2] lavu: Add AVSphericalMapping type and frame side data
On Wed, Nov 16, 2016 at 6:29 PM, Michael Niedermayer
wrote:
> On Wed, Nov 16, 2016 at 05:38:20PM -0500, Vittorio Giovara wrote:
>> On Tue, Nov 15, 2016 at 8:52 PM, James Almer wrote:
>> > On 11/15/2016 10:39 PM, Michael Niedermayer wrote:
>> >> On Tue, Nov 15, 2016 at 11:56:48AM -0500, Vittorio Giovara wrote:
>> >> [...]
>> >>> +/**
>> >>> + * This structure describes how to handle spherical videos, outlining
>> >>> + * information about projection, initial layout, and any other view
>> >>> modifier.
>> >>> + *
>> >>> + * @note The struct must be allocated with av_spherical_alloc() and
>> >>> + * its size is not a part of the public ABI.
>> >>> + */
>> >>> +typedef struct AVSphericalMapping {
>> >>> +/**
>> >>> + * Projection type.
>> >>> + */
>> >>> +enum AVSphericalProjection projection;
>> >>> +
>> >>> +/**
>> >>> + * @name Initial orientation
>> >>> + * @{
>> >>> + * These fields represent the pose values that measure the rotation
>> >>> + * transformation (in degrees) to be applied to the projection.
>> >>
>> >>> + * They are exported as 16.16 fixed point.
>> >>
>> >> why waste 7 bits of precission ?
>> >
>> > 16.16 seems to be part of the spec
>> >
>> > https://github.com/google/spatial-media/blob/master/docs/spherical-video-v2-rfc.md
>>
>> Correct, there is no point in adding more precision that the
>> underlying layer can withhold.
>
> if people prefer storing
> angle * 180 * (1<<16) / PI (using about 25bits of 32bits)
> instead of
> angle * (1<<31 - 1) / PI(using 32bits)
>
> than iam happy with that too, ive a slight preferrance for using the
> full 32bits though
I'm ok with how it is now. I think it's a reasonable compromise.
> to me the
> "underlying layer" for rotations is not some spec but reality
"Reality" still need to be transmitted and described in binary format
agreed by each party ;-)
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3 v2] mov: Export spherical information
On Wed, Nov 16, 2016 at 5:57 PM, James Almer wrote: > On 11/16/2016 12:33 PM, Vittorio Giovara wrote: >> On Tue, Nov 15, 2016 at 10:09 PM, James Almer wrote: >>> On 11/15/2016 1:56 PM, Vittorio Giovara wrote: >>> >>> Does Google offer samples to confirm this? >> >> Yep. >> >> https://transfer.sh/WOegj/motherboard-cube-v2-metadata.mp4 >> https://transfer.sh/31Zvg/motherboard-equirect-v1-metadata.mp4 >> https://transfer.sh/tn1xe/motherboard-equirect-v2-metadata.mp4 > > Nice, thanks. Although it would have been better if they had some values other > than 0 for every spherical related field. > > Do they also happen to offer webm samples with the elements detailed in this > spec? Not right now, but I'll forward your questions. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3 v2] lavu: Add AVSphericalMapping type and frame side data
On Tue, Nov 15, 2016 at 8:52 PM, James Almer wrote:
> On 11/15/2016 10:39 PM, Michael Niedermayer wrote:
>> On Tue, Nov 15, 2016 at 11:56:48AM -0500, Vittorio Giovara wrote:
>> [...]
>>> +/**
>>> + * This structure describes how to handle spherical videos, outlining
>>> + * information about projection, initial layout, and any other view
>>> modifier.
>>> + *
>>> + * @note The struct must be allocated with av_spherical_alloc() and
>>> + * its size is not a part of the public ABI.
>>> + */
>>> +typedef struct AVSphericalMapping {
>>> +/**
>>> + * Projection type.
>>> + */
>>> +enum AVSphericalProjection projection;
>>> +
>>> +/**
>>> + * @name Initial orientation
>>> + * @{
>>> + * These fields represent the pose values that measure the rotation
>>> + * transformation (in degrees) to be applied to the projection.
>>
>>> + * They are exported as 16.16 fixed point.
>>
>> why waste 7 bits of precission ?
>
> 16.16 seems to be part of the spec
>
> https://github.com/google/spatial-media/blob/master/docs/spherical-video-v2-rfc.md
Correct, there is no point in adding more precision that the
underlying layer can withhold.
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/3 v2] mov: Export spherical information
On Tue, Nov 15, 2016 at 10:09 PM, James Almer wrote:
> On 11/15/2016 1:56 PM, Vittorio Giovara wrote:
>> +static int mov_read_st3d(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>> +{
>> +AVStream *st;
>> +MOVStreamContext *sc;
>> +enum AVStereo3DType type;
>> +int mode;
>> +
>> +if (c->fc->nb_streams < 1)
>> +return 0;
>> +
>> +st = c->fc->streams[c->fc->nb_streams - 1];
>> +sc = st->priv_data;
>> +
>> +if (atom.size < 1) {
>> +av_log(c->fc, AV_LOG_ERROR, "Empty stereoscopic video box\n");
>> +return AVERROR_INVALIDDATA;
>> +}
>> +
>> +mode = avio_r8(pb);
>
> If i'm reading the spec right, st3d is a FullBox so before the data you'll
> have the box's version and flags (1 byte and 3 bytes respectively).
Oh I see, so I should check for version == 0 and read three bytes only
instead of 4.
>> +av_log(c->fc, AV_LOG_ERROR, "Missing projection header box\n");
>> +return 0;
>> +}
>> +
>> +/* 16.16 fixed point */
>> +yaw = avio_rb32(pb);
>> +pitch = avio_rb32(pb);
>> +roll = avio_rb32(pb);
>> +
>> +avio_skip(pb, size - 20);
>> +
>> +size = avio_rb32(pb);
>> +if (size > atom.size)
>> +return AVERROR_INVALIDDATA;
>> +
>> +tag = avio_rl32(pb);
>
> And all the possible cases for this one, except in here it seems to be 4
> bytes for
> version and four for flags.
>
> Does Google offer samples to confirm this?
Yep.
https://transfer.sh/WOegj/motherboard-cube-v2-metadata.mp4
https://transfer.sh/31Zvg/motherboard-equirect-v1-metadata.mp4
https://transfer.sh/tn1xe/motherboard-equirect-v2-metadata.mp4
Thanks
--
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] fate: Add h264 extradata reload tests
On Tue, Nov 15, 2016 at 8:47 PM, Michael Niedermayer wrote: > On Tue, Nov 15, 2016 at 11:18:02AM -0500, Vittorio Giovara wrote: >> --- >> This is the version without hevc testing. >> Please CC. >> Vittorio >> >> tests/fate/h264.mak | 5 + >> tests/ref/fate/h264-extradata-reload | 13 + >> 2 files changed, 18 insertions(+) >> create mode 100644 tests/ref/fate/h264-extradata-reload > > applied > > thx Thanks! Do you think you could apply the hevc extradata reset patches too (even without a test)? -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/3 v2] mov: Export spherical information
This implements Spherical Video V1 and V2, as described in the
spatial-media collection by Google.
Signed-off-by: Vittorio Giovara
---
Updated to use int32 for rotation.
Please CC.
Vittorio
libavformat/isom.h | 7 ++
libavformat/mov.c | 281 -
2 files changed, 287 insertions(+), 1 deletion(-)
diff --git a/libavformat/isom.h b/libavformat/isom.h
index 02bfedd..0fd9eb0 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -24,6 +24,9 @@
#ifndef AVFORMAT_ISOM_H
#define AVFORMAT_ISOM_H
+#include "libavutil/spherical.h"
+#include "libavutil/stereo3d.h"
+
#include "avio.h"
#include "internal.h"
#include "dv.h"
@@ -177,6 +180,10 @@ typedef struct MOVStreamContext {
int stsd_count;
int32_t *display_matrix;
+AVStereo3D *stereo3d;
+AVSphericalMapping *spherical;
+size_t spherical_size;
+
uint32_t format;
int has_sidx; // If there is an sidx entry for this stream.
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 6beb027..a017bc0 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -42,6 +42,8 @@
#include "libavutil/aes.h"
#include "libavutil/aes_ctr.h"
#include "libavutil/sha.h"
+#include "libavutil/spherical.h"
+#include "libavutil/stereo3d.h"
#include "libavutil/timecode.h"
#include "libavcodec/ac3tab.h"
#include "libavcodec/mpegaudiodecheader.h"
@@ -4497,8 +4499,230 @@ static int mov_read_tmcd(MOVContext *c, AVIOContext
*pb, MOVAtom atom)
return 0;
}
+static int mov_read_st3d(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+AVStream *st;
+MOVStreamContext *sc;
+enum AVStereo3DType type;
+int mode;
+
+if (c->fc->nb_streams < 1)
+return 0;
+
+st = c->fc->streams[c->fc->nb_streams - 1];
+sc = st->priv_data;
+
+if (atom.size < 1) {
+av_log(c->fc, AV_LOG_ERROR, "Empty stereoscopic video box\n");
+return AVERROR_INVALIDDATA;
+}
+
+mode = avio_r8(pb);
+switch (mode) {
+case 0:
+type = AV_STEREO3D_2D;
+break;
+case 1:
+type = AV_STEREO3D_TOPBOTTOM;
+break;
+case 2:
+type = AV_STEREO3D_SIDEBYSIDE;
+break;
+default:
+av_log(c->fc, AV_LOG_WARNING, "Unknown st3d mode value %d\n", mode);
+return 0;
+}
+
+sc->stereo3d = av_stereo3d_alloc();
+if (!sc->stereo3d)
+return AVERROR(ENOMEM);
+
+sc->stereo3d->type = type;
+return 0;
+}
+
+static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+AVStream *st;
+MOVStreamContext *sc;
+int size;
+int32_t yaw, pitch, roll;
+uint32_t tag;
+unsigned l, t, r, b;
+enum AVSphericalProjection projection;
+
+if (c->fc->nb_streams < 1)
+return 0;
+
+st = c->fc->streams[c->fc->nb_streams - 1];
+sc = st->priv_data;
+
+if (atom.size < 4) {
+av_log(c->fc, AV_LOG_ERROR, "Empty spherical video box\n");
+return AVERROR_INVALIDDATA;
+}
+
+size = avio_rb32(pb);
+if (size > atom.size)
+return AVERROR_INVALIDDATA;
+
+tag = avio_rl32(pb);
+if (tag != MKTAG('s','v','h','d')) {
+av_log(c->fc, AV_LOG_ERROR, "Missing spherical video header\n");
+return 0;
+}
+avio_skip(pb, size - 8); /* metadata_source */
+
+size = avio_rb32(pb);
+if (size > atom.size)
+return AVERROR_INVALIDDATA;
+
+tag = avio_rl32(pb);
+if (tag != MKTAG('p','r','o','j')) {
+av_log(c->fc, AV_LOG_ERROR, "Missing projection box\n");
+return 0;
+}
+
+size = avio_rb32(pb);
+if (size > atom.size)
+return AVERROR_INVALIDDATA;
+
+tag = avio_rl32(pb);
+if (tag != MKTAG('p','r','h','d')) {
+av_log(c->fc, AV_LOG_ERROR, "Missing projection header box\n");
+return 0;
+}
+
+/* 16.16 fixed point */
+yaw = avio_rb32(pb);
+pitch = avio_rb32(pb);
+roll = avio_rb32(pb);
+
+avio_skip(pb, size - 20);
+
+size = avio_rb32(pb);
+if (size > atom.size)
+return AVERROR_INVALIDDATA;
+
+tag = avio_rl32(pb);
+switch (tag) {
+case MKTAG('c','b','m','p'):
+projection = AV_SPHERICAL_CUBEMAP;
+avio_skip(pb, 4); /* layout */
+l = t = r = b = avio_rb32(pb);
+break;
+case MKTAG('e','q','u','i'):
+projection = AV_SPHERICAL_EQUIRECTANGULAR;
+t = avio_rb32(pb);
+b = avio_rb32(pb);
+l = avio_rb32(pb);
+r = avio_rb32(pb);
+break;
+defaul
[FFmpeg-devel] [PATCH 1/3 v2] lavu: Add AVSphericalMapping type and frame side data
While no decoder currently exports spherical information, this type
represents a frame property that has to be passed through from container
to frames.
Signed-off-by: Vittorio Giovara
---
Updated to use int32 for rotation.
Please CC.
Vittorio
libavutil/Makefile| 2 +
libavutil/avutil.h| 6 +++
libavutil/frame.h | 8 ++-
libavutil/spherical.c | 34
libavutil/spherical.h | 146 ++
5 files changed, 195 insertions(+), 1 deletion(-)
create mode 100644 libavutil/spherical.c
create mode 100644 libavutil/spherical.h
diff --git a/libavutil/Makefile b/libavutil/Makefile
index 0fa90fe..c5af79a 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -63,6 +63,7 @@ HEADERS = adler32.h
\
samplefmt.h \
sha.h \
sha512.h \
+ spherical.h \
stereo3d.h\
threadmessage.h \
time.h\
@@ -140,6 +141,7 @@ OBJS = adler32.o
\
samplefmt.o \
sha.o\
sha512.o \
+ spherical.o \
stereo3d.o \
threadmessage.o \
time.o \
diff --git a/libavutil/avutil.h b/libavutil/avutil.h
index 29dd830..e9aaa03 100644
--- a/libavutil/avutil.h
+++ b/libavutil/avutil.h
@@ -118,6 +118,12 @@
*
* @}
*
+ * @defgroup lavu_video Video related
+ *
+ * @{
+ *
+ * @}
+ *
* @defgroup lavu_audio Audio related
*
* @{
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 8e51361..b450092 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -120,7 +120,13 @@ enum AVFrameSideDataType {
* The GOP timecode in 25 bit timecode format. Data format is 64-bit
integer.
* This is set on the first frame of a GOP that has a temporal reference
of 0.
*/
-AV_FRAME_DATA_GOP_TIMECODE
+AV_FRAME_DATA_GOP_TIMECODE,
+
+/**
+ * The data represents the AVSphericalMapping structure defined in
+ * libavutil/spherical.h.
+ */
+AV_FRAME_DATA_SPHERICAL,
};
enum AVActiveFormatDescription {
diff --git a/libavutil/spherical.c b/libavutil/spherical.c
new file mode 100644
index 000..816452c
--- /dev/null
+++ b/libavutil/spherical.c
@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2016 Vittorio Giovara
+ *
+ * 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 "mem.h"
+#include "spherical.h"
+
+AVSphericalMapping *av_spherical_alloc(size_t *size)
+{
+AVSphericalMapping *spherical = av_mallocz(sizeof(AVSphericalMapping));
+if (!spherical)
+return NULL;
+
+if (size)
+*size = sizeof(*spherical);
+
+return spherical;
+}
diff --git a/libavutil/spherical.h b/libavutil/spherical.h
new file mode 100644
index 000..6d5a316
--- /dev/null
+++ b/libavutil/spherical.h
@@ -0,0 +1,146 @@
+/*
+ * Copyright (c) 2016 Vittorio Giovara
+ *
+ * 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 L
[FFmpeg-devel] [PATCH 2/3 v2] lavc: Add spherical packet side data API
Signed-off-by: Vittorio Giovara
---
Updated to use int32 for rotation.
Please CC.
Vittorio
ffprobe.c | 18 ++
libavcodec/avcodec.h | 8 +++-
libavcodec/avpacket.c | 1 +
libavcodec/utils.c| 1 +
libavformat/dump.c| 36
5 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/ffprobe.c b/ffprobe.c
index a2980b3..839963a 100644
--- a/ffprobe.c
+++ b/ffprobe.c
@@ -37,6 +37,7 @@
#include "libavutil/hash.h"
#include "libavutil/opt.h"
#include "libavutil/pixdesc.h"
+#include "libavutil/spherical.h"
#include "libavutil/stereo3d.h"
#include "libavutil/dict.h"
#include "libavutil/intreadwrite.h"
@@ -1783,6 +1784,23 @@ static void print_pkt_side_data(WriterContext *w,
const AVStereo3D *stereo = (AVStereo3D *)sd->data;
print_str("type", av_stereo3d_type_name(stereo->type));
print_int("inverted", !!(stereo->flags & AV_STEREO3D_FLAG_INVERT));
+} else if (sd->type == AV_PKT_DATA_SPHERICAL) {
+const AVSphericalMapping *spherical = (AVSphericalMapping
*)sd->data;
+if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR)
+print_str("projection", "equirectangular");
+else if (spherical->projection == AV_SPHERICAL_CUBEMAP)
+print_str("projection", "cubemap");
+else
+print_str("projection", "unknown");
+
+print_int("yaw", ((double)spherical->yaw) / (1 << 16));
+print_int("pitch", ((double)spherical->pitch) / (1 << 16));
+print_int("roll", ((double)spherical->roll) / (1 << 16));
+
+print_int("left", spherical->left_offset);
+print_int("top", spherical->top_offset);
+print_int("right", spherical->right_offset);
+print_int("bottom", spherical->bottom_offset);
}
writer_print_section_footer(w);
}
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 22f..9497cc3 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1536,7 +1536,13 @@ enum AVPacketSideDataType {
* should be associated with a video stream and containts data in the form
* of the AVMasteringDisplayMetadata struct.
*/
-AV_PKT_DATA_MASTERING_DISPLAY_METADATA
+AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
+
+/**
+ * This side data should be associated with a video stream and corresponds
+ * to the AVSphericalMapping structure.
+ */
+AV_PKT_DATA_SPHERICAL,
};
#define AV_PKT_DATA_QUALITY_FACTOR AV_PKT_DATA_QUALITY_STATS //DEPRECATED
diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index c3f871c..a799610 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -371,6 +371,7 @@ const char *av_packet_side_data_name(enum
AVPacketSideDataType type)
case AV_PKT_DATA_METADATA_UPDATE:return "Metadata Update";
case AV_PKT_DATA_MPEGTS_STREAM_ID: return "MPEGTS Stream ID";
case AV_PKT_DATA_MASTERING_DISPLAY_METADATA: return "Mastering display
metadata";
+case AV_PKT_DATA_SPHERICAL: return "Spherical mapping";
}
return NULL;
}
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index d6dca18..89a12c6 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -762,6 +762,7 @@ int ff_init_buffer_info(AVCodecContext *avctx, AVFrame
*frame)
} sd[] = {
{ AV_PKT_DATA_REPLAYGAIN ,AV_FRAME_DATA_REPLAYGAIN },
{ AV_PKT_DATA_DISPLAYMATRIX, AV_FRAME_DATA_DISPLAYMATRIX
},
+{ AV_PKT_DATA_SPHERICAL, AV_FRAME_DATA_SPHERICAL },
{ AV_PKT_DATA_STEREO3D, AV_FRAME_DATA_STEREO3D },
{ AV_PKT_DATA_AUDIO_SERVICE_TYPE,
AV_FRAME_DATA_AUDIO_SERVICE_TYPE },
{ AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
AV_FRAME_DATA_MASTERING_DISPLAY_METADATA },
diff --git a/libavformat/dump.c b/libavformat/dump.c
index cd14625..2dd7a0a 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -31,6 +31,7 @@
#include "libavutil/opt.h"
#include "libavutil/avstring.h"
#include "libavutil/replaygain.h"
+#include "libavutil/spherical.h"
#include "libavutil/stereo3d.h"
#include "avformat.h"
@@ -342,6 +343,37 @@ static void dump_mastering_display_metadata(void *ctx,
AVPacketSideData* sd) {
av_q2d(metadata->min_luminance), av_q2d(metadata->max_luminance));
}
+static void dump_spherical(void *ctx, AVPacketSideData *sd)
+{
+AVSphericalMapping *spherical = (AVSphericalMapping *)sd->
[FFmpeg-devel] [PATCHv5] mov: Evaluate the movie display matrix
This matrix needs to be applied after all others have (currently only
display matrix from trak), but cannot be handled in movie box, since
streams are not allocated yet. So store it in main context, and apply
it when appropriate, that is after parsing the tkhd one.
Signed-off-by: Vittorio Giovara
---
Fixed a boolean operation. No other changes, ready to be committed.
Please CC.
Vittorio
libavformat/isom.h | 1 +
libavformat/mov.c| 48 +---
tests/fate/mov.mak | 5 -
tests/ref/fate/mov-displaymatrix | 10 +
4 files changed, 50 insertions(+), 14 deletions(-)
create mode 100644 tests/ref/fate/mov-displaymatrix
diff --git a/libavformat/isom.h b/libavformat/isom.h
index d684502..02bfedd 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -240,6 +240,7 @@ typedef struct MOVContext {
uint8_t *decryption_key;
int decryption_key_len;
int enable_drefs;
+int32_t movie_display_matrix[3][3]; ///< display matrix from mvhd
} MOVContext;
int ff_mp4_read_descr_len(AVIOContext *pb);
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 8d6cc12..b7d0b12 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1239,6 +1239,7 @@ static int mov_read_mdhd(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
static int mov_read_mvhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
{
+int i;
int64_t creation_time;
int version = avio_r8(pb); /* version */
avio_rb24(pb); /* flags */
@@ -1269,7 +1270,12 @@ static int mov_read_mvhd(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
avio_skip(pb, 10); /* reserved */
-avio_skip(pb, 36); /* display matrix */
+/* movie display matrix, store it in main context and use it later on */
+for (i = 0; i < 3; i++) {
+c->movie_display_matrix[i][0] = avio_rb32(pb); // 16.16 fixed point
+c->movie_display_matrix[i][1] = avio_rb32(pb); // 16.16 fixed point
+c->movie_display_matrix[i][2] = avio_rb32(pb); // 2.30 fixed point
+}
avio_rb32(pb); /* preview time */
avio_rb32(pb); /* preview duration */
@@ -3847,12 +3853,22 @@ static int mov_read_meta(MOVContext *c, AVIOContext
*pb, MOVAtom atom)
return 0;
}
+// return 1 when matrix is identity, 0 otherwise
+#define IS_MATRIX_IDENT(matrix)\
+( (matrix)[0][0] == (1 << 16) && \
+ (matrix)[1][1] == (1 << 16) && \
+ (matrix)[2][2] == (1 << 30) && \
+ !(matrix)[0][1] && !(matrix)[0][2] && \
+ !(matrix)[1][0] && !(matrix)[1][2] && \
+ !(matrix)[2][0] && !(matrix)[2][1])
+
static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
{
-int i;
+int i, j, e;
int width;
int height;
int display_matrix[3][3];
+int res_display_matrix[3][3] = { { 0 } };
AVStream *st;
MOVStreamContext *sc;
int version;
@@ -3902,15 +3918,20 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext
*pb, MOVAtom atom)
sc->width = width >> 16;
sc->height = height >> 16;
-// save the matrix and add rotate metadata when it is not the default
-// identity
-if (display_matrix[0][0] != (1 << 16) ||
-display_matrix[1][1] != (1 << 16) ||
-display_matrix[2][2] != (1 << 30) ||
-display_matrix[0][1] || display_matrix[0][2] ||
-display_matrix[1][0] || display_matrix[1][2] ||
-display_matrix[2][0] || display_matrix[2][1]) {
-int i, j;
+// apply the moov display matrix (after the tkhd one)
+for (i = 0; i < 3; i++) {
+const int sh[3] = { 16, 16, 30 };
+for (j = 0; j < 3; j++) {
+for (e = 0; e < 3; e++) {
+res_display_matrix[i][j] +=
+((int64_t) display_matrix[i][e] *
+ c->movie_display_matrix[e][j]) >> sh[e];
+}
+}
+}
+
+// save the matrix when it is not the default identity
+if (!IS_MATRIX_IDENT(res_display_matrix)) {
double rotate;
av_freep(&sc->display_matrix);
@@ -3920,7 +3941,7 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
for (i = 0; i < 3; i++)
for (j = 0; j < 3; j++)
-sc->display_matrix[i * 3 + j] = display_matrix[i][j];
+sc->display_matrix[i * 3 + j] = res_display_matrix[i][j];
rotate = av_display_rotation_get(sc->display_matrix);
if (!isnan(rotate)) {
@@ -3939,7 +3960,8 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
double disp_transform[2];
for (i = 0; i < 2; i++)
-disp_transform[i] = hypot(display_matrix[i][0],
display_matrix[i][1]);
+disp_transform[i] = hypot(sc->display_matrix[0 + i],
+
[FFmpeg-devel] [PATCH] fate: Add h264 extradata reload tests
--- This is the version without hevc testing. Please CC. Vittorio tests/fate/h264.mak | 5 + tests/ref/fate/h264-extradata-reload | 13 + 2 files changed, 18 insertions(+) create mode 100644 tests/ref/fate/h264-extradata-reload diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak index b4d7f7a..f8ef3f4 100644 --- a/tests/fate/h264.mak +++ b/tests/fate/h264.mak @@ -196,6 +196,10 @@ FATE_H264 := $(FATE_H264:%=fate-h264-conformance-%) \ FATE_H264-$(call DEMDEC, H264, H264) += $(FATE_H264) FATE_H264-$(call DEMDEC, MOV, H264) += fate-h264-crop-to-container + +# this sample has two stsd entries and needs to reload extradata +FATE_H264-$(call DEMDEC, MOV, H264) += fate-h264-extradata-reload + FATE_H264-$(call DEMDEC, MOV, H264) += fate-h264-interlace-crop # this sample has invalid reference list modification, but decodes fine @@ -408,6 +412,7 @@ fate-h264-conformance-sva_nl2_e: CMD = framecrc -vsync drop -i fate-h264-bsf-mp4toannexb:CMD = md5 -i $(TARGET_SAMPLES)/h264/interlaced_crop.mp4 -vcodec copy -f h264 fate-h264-crop-to-container: CMD = framemd5 -i $(TARGET_SAMPLES)/h264/crop-to-container-dims-canon.mov +fate-h264-extradata-reload: CMD = framemd5 -i $(TARGET_SAMPLES)/h264/extradata-reload-multi-stsd.mov fate-h264-extreme-plane-pred: CMD = framemd5 -i $(TARGET_SAMPLES)/h264/extreme-plane-pred.h264 fate-h264-interlace-crop: CMD = framecrc -i $(TARGET_SAMPLES)/h264/interlaced_crop.mp4 -vframes 3 fate-h264-lossless: CMD = framecrc -i $(TARGET_SAMPLES)/h264/lossless.h264 diff --git a/tests/ref/fate/h264-extradata-reload b/tests/ref/fate/h264-extradata-reload new file mode 100644 index 000..c70f805 --- /dev/null +++ b/tests/ref/fate/h264-extradata-reload @@ -0,0 +1,13 @@ +#format: frame checksums +#version: 2 +#hash: MD5 +#tb 0: 1/25 +#media_type 0: video +#codec_id 0: rawvideo +#dimensions 0: 256x128 +#sar 0: 1/1 +#stream#, dts,pts, duration, size, hash +0, 0, 0,1,49152, ae09c88e87e3ea0aa8ad267ee91222e5 +0, 1, 1,1,49152, 7ccd8321a6e23ae1f82bd323c8376524 +0, 2, 2,1,49152, 8ed93ab585ff9848801cc28b75b5e12d +0, 3, 3,1,49152, 0049913870ddb62ffc535282018766f4 -- 2.10.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] lavu: Add AVSphericalMapping type and frame side data
On Mon, Nov 14, 2016 at 8:55 PM, Michael Niedermayer wrote: > On Mon, Nov 14, 2016 at 04:55:36PM -0500, Vittorio Giovara wrote: >> On Sun, Nov 13, 2016 at 11:57 AM, Michael Niedermayer >> wrote: >> > On Sun, Nov 13, 2016 at 10:18:18AM +0100, Michael Niedermayer wrote: >> >> On Sat, Nov 12, 2016 at 10:05:22PM -0500, Vittorio Giovara wrote: >> >> [...] >> >> > So I really do not see a use case for using int64 here. >> >> >> >> then you can use int32, less than int32 is too little if for example >> >> you wnat the precission to be sufficient to know where a tele lens >> >> points with pixel precission and have a bit precission headroom >> >> Hi Michael, thanks for keeping me in CC. >> I understand the problem, but this is a solution for a issue that is >> non-existent. > > The problem of difficut to test code is real and existing in general. > Encoders&muxers using floats/doubles can not be tested as completely as > Encoders&muxers not using floats unless they by chance give binary > identical results on all platforms. That's not the problem you referred to in the previous email. > Muxers&encoders would use/write the rotation side data > > Similarly ffprobe would at some point print this data, the text > printout of doubles has a good chance to not match exactly between > platforms. ffprobe does that in 2/3, but right now it's limited to int. >> There is no application or usecase for "pixel perfect" >> precision, the rendering of a spherical video will distort the video >> surface in order to map the flat surface to a sphere, and it is >> impossible to predict where this operation will move pixels to. I >> believe this is why this specification describes the initial >> orientation as rotation degrees rather than pixel offsets. > > I referred to pixels only to show that 32bit floats are insufficiently > precisse in some cases. I still don't like it, but I'll just export the original 16.16 fixed point value and let the user deal with it then. Thanks for the reviews. -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
