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

Yuval Adam yuv.adm at gmail.com
Thu Jan 5 11:02:44 CET 2012


On Wed, Jan 4, 2012 at 8:51 PM, Nicolas George <
nicolas.george at normalesup.org> wrote:

> 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.
>

Thanks, I'll send it directly from git send-email next time.


>
> >   * @param ts frame timestamp in seconds
>
> That description is no longer accurate, I think.
>

True, it is now in sub-seconds.


>
> >   * @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.
>

Yep, thanks for the catch.


>
> 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.
>

That's exactly what I was thinking, I just didn't know if it makes sense to
do that.


>
> > -    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.
>

I'll do rebasing on the whole thing so that the next patch will be unified.
Thanks for the feedback!


>
> Regards,
>
> --
>  Nicolas George
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
>
> iEYEARECAAYFAk8En80ACgkQsGPZlzblTJOHMgCgvmAxM9GtLOwi6fg3OwShyyrM
> qbgAn1Q2NDw0FfRS2ATGU9/7PuP0XvcW
> =knd/
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>


-- 
Yuval Adam
http://y3xz.com


More information about the ffmpeg-devel mailing list