libavformat/mov: Add support for exporting poster time.

[brycechesternewman]

Export the poster_time_location if available.
The poster_time_location is calculated using
the poster_time / time_scale = X seconds.
The value of poster_time_location indicates
where in the video the poster frame is.

Addresses feedback
from https://www.mail-archive.com/ffmpeg-devel@ffmpeg.org/msg138122.html.

Signed-off-by: Bryce Chester Newman bryce.newman@gettyimages.com
cc: Bryce Newman bryce.newman@gettyimages.com
cc: "zhilizhao(赵志立)" quinkblack@foxmail.com

[softworkz]

👍  Now it should work...

[brycechesternewman]

/submit

[ffmpeg-codebot]

Submitted as pull.41.ffstaging.FFmpeg.1664813294282.ffmpegagent@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-41/brycechesternewman/add_poster_time_location_mov-v1

To fetch this version to local tag pr-ffstaging-41/brycechesternewman/add_poster_time_location_mov-v1:

git fetch --no-tags https://github.com/ffstaging/FFmpeg tag pr-ffstaging-41/brycechesternewman/add_poster_time_location_mov-v1

[brycechesternewman]

@softworkz Doesn't this seem odd https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=&submitter=Bryce&state=&q=&archive=&delegate=? Notice all the commits I did not do.

[softworkz]

@softworkz Doesn't this seem odd patchwork.ffmpeg.org/project/ffmpeg/list/?series=&submitter=Bryce&state=&q=&archive=&delegate=? Notice all the commits I did not do.

Yes, this is a bug in patchwork. Those are all mine. On each submission, Patchwork uses the submitter's name for all submissions though the ffmpeg codebot.

Andriy Gelman had promised to look at it at some time.

[brycechesternewman]

/submit

[ffmpeg-codebot]

Submitted as pull.41.v2.ffstaging.FFmpeg.1664979908.ffmpegagent@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-41/brycechesternewman/add_poster_time_location_mov-v2

To fetch this version to local tag pr-ffstaging-41/brycechesternewman/add_poster_time_location_mov-v2:

git fetch --no-tags https://github.com/ffstaging/FFmpeg tag pr-ffstaging-41/brycechesternewman/add_poster_time_location_mov-v2

[ffmpeg-codebot]

User Bryce Newman <bryce.newman@gettyimages.com> has been added to the cc: list.

[ffmpeg-codebot]

On the FFmpeg mailing list, "zhilizhao(赵志立)" wrote (reply to this):

> On Oct 5, 2022, at 22:25, Bryce Chester Newman <ffmpegagent@gmail.com> wrote:
> 
> From: Bryce Chester Newman <bryce.newman@gettyimages.com>
> 
> Change demuxer option name from
> poster_time_location
> to export_poster_time_location.
> 
> Export the poster_time_location if available.
> The poster_time_location is calculated using
> the poster_time / time_scale = X seconds.
> The value of poster_time_location indicates
> where in the video the poster frame is.
> 
> Addresses feedback
> from https://www.mail-archive.com/ffmpeg-devel@ffmpeg.org/msg138122.html.
> 
> Signed-off-by: Bryce Chester Newman bryce.newman@gettyimages.com
> ---
> doc/demuxers.texi  | 4 ++--
> libavformat/isom.h | 2 +-
> libavformat/mov.c  | 4 ++--
> 3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> index b1f4926c40..447287357d 100644
> --- a/doc/demuxers.texi
> +++ b/doc/demuxers.texi
> @@ -750,10 +750,10 @@ cast to int32 are used to adjust onward dts.
> Unit is the track time scale. Range is 0 to UINT_MAX. Default is @code{UINT_MAX - 48000*10} which allows upto
> a 10 second dts correction for 48 kHz audio streams while accommodating 99.9% of @code{uint32} range.
> 
> -@item poster_time_location
> +@item export_poster_time_location
> Export the poster_time_location if available.
> The poster_time_location is calculated using the poster_time / time_scale = X seconds.
> -The value of poster_time_location indicates where in the video the poster frame is.
> +The value of the poster_time_location key indicates where in the video the poster frame is.
> Default is false.
> @end table
> 
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index fb3d8d5618..f621abec76 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -326,7 +326,7 @@ typedef struct MOVContext {
>         int64_t extent_offset;
>     } *avif_info;
>     int avif_info_size;
> -    int poster_time_location;
> +    int export_poster_time_location;
> } MOVContext;
> 
> int ff_mp4_read_descr_len(AVIOContext *pb);
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index b914bbc96a..be939f6cc2 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -1542,7 +1542,7 @@ static int mov_read_mvhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>     avio_rb32(pb); /* current time */
>     avio_rb32(pb); /* next track ID */
> 
> -    if(c->poster_time_location && poster_time && c->time_scale && c->time_scale > 0) {
> +    if(c->export_poster_time_location && poster_time && c->time_scale && c->time_scale > 0) {
>         av_log(c->fc, AV_LOG_TRACE, "poster_time = %i, time_scale = %i\n", poster_time, c->time_scale);
>         char buffer[32];
>         int poster_time_location = poster_time / c->time_scale;
> @@ -9123,7 +9123,7 @@ static const AVOption mov_options[] = {
>     { "enable_drefs", "Enable external track support.", OFFSET(enable_drefs), AV_OPT_TYPE_BOOL,
>         {.i64 = 0}, 0, 1, FLAGS },
>     { "max_stts_delta", "treat offsets above this value as invalid", OFFSET(max_stts_delta), AV_OPT_TYPE_INT, {.i64 = UINT_MAX-48000*10 }, 0, UINT_MAX, .flags = AV_OPT_FLAG_DECODING_PARAM },
> -    { "poster_time_location", "Export the poster time location.", OFFSET(poster_time_location), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS | AV_OPT_FLAG_EXPORT },
> +    { "export_poster_time_location", "Export the poster time location.", OFFSET(export_poster_time_location), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS | AV_OPT_FLAG_EXPORT },
>     { NULL },
> };
> 

Firstly, the patch should be based on master.

Secondly, it’s too ad hoc. I don’t see a strong reason to export such information.
Those fields only defined by quicktime, not defined by ISO base media file format.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[brycechesternewman]

The patch was based off master over 3 months ago. @softworkz Is there something that is not correctly happening when pull requests are created from this repository? What can we do to make sure the patch is from master?

What specifically is ad hoc about the change? We have a very good reason to export the information since extraction of that value is a business requirement on our end.

You are correct the field is only defined in QuickTime. Is there a suggestion that you can provide that would limit this change only to export if the video is QuickTime? Would that help allow this change to be approved?

[ffmpeg-codebot]

User "zhilizhao(赵志立)" <quinkblack@foxmail.com> has been added to the cc: list.