[FFmpeg-devel] [PATCH] Support 64-bit integers for av_get_frame_filename2()

[Michael Niedermayer]

On Sat, Aug 25, 2018 at 03:30:48PM +0200, Marton Balint wrote:
> 
> 
> On Sat, 25 Aug 2018, Michael Niedermayer wrote:
> 
> >On Fri, Aug 24, 2018 at 04:33:18PM -0300, James Almer wrote:
> >>On 8/24/2018 3:56 PM, Devin Heitmueller wrote:
> >>>Create a new av_get_frame_filename3() API which is just like the
> >>>previous version but accepts a 64-bit integer for the "number"
> >>>argument.  This is useful in cases where you want to put the
> >>>original PTS into the filename (which can be larger than 32-bits).
> >>>
> >>>Tested with:
> >>>
> >>>./ffmpeg -copyts -vsync 0 -i foo.ts -frame_pts 1 -enc_time_base -1 foo_%d.png
> >>>
> >>>Signed-off-by: Devin Heitmueller <dheitmueller at ltnglobal.com>
> >>>---
> >>> libavformat/avformat.h | 2 ++
> >>> libavformat/img2enc.c  | 2 +-
> >>> libavformat/utils.c    | 9 +++++++--
> >>> 3 files changed, 10 insertions(+), 3 deletions(-)
> >>
> >>Missing APIChanges entry and libavformat minor version bump.
> >>
> >>>
> >>>diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> >>>index fdaffa5bf4..c358a9a71e 100644
> >>>--- a/libavformat/avformat.h
> >>>+++ b/libavformat/avformat.h
> >>>@@ -2896,6 +2896,8 @@ void av_dump_format(AVFormatContext *ic,
> >>>  * @param flags AV_FRAME_FILENAME_FLAGS_*
> >>>  * @return 0 if OK, -1 on format error
> >>>  */
> >>>+int av_get_frame_filename3(char *buf, int buf_size,
> >>>+                          const char *path, int64_t number, int flags);
> >>
> >>Make buf_size of size_t type while at it.
> >>
> >>> int av_get_frame_filename2(char *buf, int buf_size,
> >>>                           const char *path, int number, int flags);
> >>>
> >>>diff --git a/libavformat/img2enc.c b/libavformat/img2enc.c
> >>>index a09cc8ec50..414eb827e2 100644
> >>>--- a/libavformat/img2enc.c
> >>>+++ b/libavformat/img2enc.c
> >>>@@ -101,7 +101,7 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt)
> >>>                 return AVERROR(EINVAL);
> >>>             }
> >>>         } else if (img->frame_pts) {
> >>>-            if (av_get_frame_filename2(filename, sizeof(filename), img->path, pkt->pts, AV_FRAME_FILENAME_FLAGS_MULTIPLE) < 0) {
> >>>+            if (av_get_frame_filename3(filename, sizeof(filename), img->path, pkt->pts, AV_FRAME_FILENAME_FLAGS_MULTIPLE) < 0) {
> >>>                 av_log(s, AV_LOG_ERROR, "Cannot write filename by pts of the frames.");
> >>>                 return AVERROR(EINVAL);
> >>>             }
> >>>diff --git a/libavformat/utils.c b/libavformat/utils.c
> >>>index b0b5e164a6..d9d4d38a44 100644
> >>>--- a/libavformat/utils.c
> >>>+++ b/libavformat/utils.c
> >>>@@ -4666,7 +4666,7 @@ uint64_t ff_get_formatted_ntp_time(uint64_t ntp_time_us)
> >>>     return ntp_ts;
> >>> }
> >>>
> >>>-int av_get_frame_filename2(char *buf, int buf_size, const char *path, int number, int flags)
> >>>+int av_get_frame_filename3(char *buf, int buf_size, const char *path, int64_t number, int flags)
> >>> {
> >>>     const char *p;
> >>>     char *q, buf1[20], c;
> >>>@@ -4696,7 +4696,7 @@ int av_get_frame_filename2(char *buf, int buf_size, const char *path, int number
> >>>                 percentd_found = 1;
> >>>                 if (number < 0)
> >>>                     nd += 1;
> >>>-                snprintf(buf1, sizeof(buf1), "%0*d", nd, number);
> >>>+                snprintf(buf1, sizeof(buf1), "%0*" PRId64, nd, number);
> >>
> >>SCNd64.
> >>
> >>>                 len = strlen(buf1);
> >>>                 if ((q - buf + len) > buf_size - 1)
> >>>                     goto fail;
> >>>@@ -4721,6 +4721,11 @@ fail:
> >>>     return -1;
> >>> }
> >>>
> >>>+int av_get_frame_filename2(char *buf, int buf_size, const char *path, int number, int flags)
> >>>+{
> >>>+    return av_get_frame_filename3(buf, buf_size, path, number, flags);
> >>>+}
> >>>+
> >>> int av_get_frame_filename(char *buf, int buf_size, const char *path, int number)
> >>> {
> >>>     return av_get_frame_filename2(buf, buf_size, path, number, 0);
> >>>
> >>
> >>No opinion on the addition, so wait for someone else to review as well.
> >
> >i think it makes sense
> >
> >also the old function should be deprecated. I think theres no reason to
> >use the old one if the 64bit one is added. We could refrain from removing
> >the old though for longer as it has no risk of breaking. But deprecation
> >would notify users that we have something newer/better to replace it
> 
> Well, actually av_get_frame_filename3 should be deprecated too soon, because
> users should not use these functions because they limit the file path to a
> fixed length. An AVBPrintf based replacement should be used instead in the
> long run.

If people prefer AVBPrintf over PATH_MAX or strlen(input) + X, then sure



[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180825/9672fcd2/attachment.sig>