[FFmpeg-devel] [PATCH 2/2] img2: revised timestamp in output pattern patch

[Nicolas George]

Le quintidi 15 nivôse, an CCXX, Yuval Adam a écrit :
> @@ -2080,8 +2088,8 @@ attribute_deprecated int find_info_tag(char *arg, int
> arg_size, const char *tag1

Your patch got word-wrapped, and is therefore unusable. You seem to be using
GMail as a webmail. To send patches, rather use it as a SMTP server.

>   * @param ts frame timestamp in seconds

That description is no longer accurate, I think.

>   * @return 0 if OK, -1 on format error
>   */
> -int av_get_frame_filename(char *buf, int buf_size,
> -                          const char *path, int number, int ts);
> +int av_get_frame_filename2(char *buf, int buf_size,
> +                           const char *path, int number, int ts);

Subsecond timestamps should be int64_t, or they will wrap.

Do we really want that as a public function? I find it rather specific, and
Google Code Search shows no one uses it. We could deprecate it and replace
it by a private ff_get_frame_filename, so we will not have to care about
compatibility.

And if we make it public, I am a little unsure about just adding the
timestamp. Adding the whole AVFrame structure seems nicer: that way, you can
add new formatting tags without creating a new function.

> -    int hours, mins, secs;
> +    int hours, mins, secs, ms;

It looks that this patch was made on top of the previous one. You need to
squash them together, either by using git commit --amend when committing or
with git rebase -i.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120104/7f4b44f8/attachment.asc>