diff mbox

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

Message ID 20180824185603.19623-1-dheitmueller@ltnglobal.com
State New
Headers show

Commit Message

Devin Heitmueller Aug. 24, 2018, 6:56 p.m. UTC
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@ltnglobal.com>
---
 libavformat/avformat.h | 2 ++
 libavformat/img2enc.c  | 2 +-
 libavformat/utils.c    | 9 +++++++--
 3 files changed, 10 insertions(+), 3 deletions(-)

Comments

James Almer Aug. 24, 2018, 7:33 p.m. UTC | #1
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@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.
Michael Niedermayer Aug. 25, 2018, 1:06 p.m. UTC | #2
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@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

thx

[...]
Marton Balint Aug. 25, 2018, 1:30 p.m. UTC | #3
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@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.

On the other hand that is a bit more work, so I am fine with the patch as 
is.

Thanks,
Marton
James Almer Aug. 25, 2018, 1:59 p.m. UTC | #4
On 8/25/2018 10:30 AM, 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@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.
> 
> On the other hand that is a bit more work, so I am fine with the patch
> as is.

No, adding a function that will be obsolete from day 1 is not a good
idea. Adding API is simple, but removing it takes years. So lets instead
add one using AVBPrintf that will be definite.

Also not called whatever3 while at it, if possible.

> 
> Thanks,
> Marton
Michael Niedermayer Aug. 25, 2018, 4:24 p.m. UTC | #5
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@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



[...]
diff mbox

Patch

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);
 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);
                 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);