On Sat, 28 May 2011 11:42:55 +0200, Stefano Sabatini 
<[email protected]> wrote:
> On date Saturday 2011-05-28 11:21:41 +0200, Anton Khirnov encoded: 
> > On Sat, 28 May 2011 10:00:14 +0200, Stefano Sabatini 
> > <[email protected]> wrote:
> > > On date Saturday 2011-05-28 07:51:22 +0200, Anton Khirnov encoded:
> > > > ---
> > > >  libavdevice/bktr.c |   24 +++++++++++++-----------
> > > >  1 files changed, 13 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/libavdevice/bktr.c b/libavdevice/bktr.c
> > > > index e8ff557..8e3b271 100644
> > > > --- a/libavdevice/bktr.c
> > > > +++ b/libavdevice/bktr.c
> > > > @@ -54,11 +54,10 @@ typedef struct {
> > > >      int video_fd;
> > > >      int tuner_fd;
> > > >      int width, height;
> > > > -    int frame_rate;
> > > > -    int frame_rate_base;
> > > >      uint64_t per_frame;
> > > >      int standard;
> > > >      char *video_size; /**< String describing video size, set by a 
> > > > private option. */
> > > > +    char *framerate;  /**< Set by a private option. */
> > > >  } VideoData;
> > > >  
> > > >  
> > > > @@ -249,8 +248,7 @@ static int grab_read_header(AVFormatContext *s1, 
> > > > AVFormatParameters *ap)
> > > >      VideoData *s = s1->priv_data;
> > > >      AVStream *st;
> > > >      int width, height;
> > > > -    int frame_rate;
> > > > -    int frame_rate_base;
> > > > +    AVRational fps;
> > > 
> > > Please let's keep consistency between related fields, in this specific
> > > case I'd do:
> > > 
> > > char       *video_rate_string;
> > > AVRational  video_rate;
> > 
> > Wasn't it you who said i should use variable names same as option
> > names? Make up your mind =p.
> > And those aren't "related fields" anyway, one is just a local variable
> > so there's no harm in making its name shorter.
> > 
> > > 
> > > Nit, maybe "video_rate" for consistency, and missing help string, I
> > > think:
> > > "A string describing video frame rate, such as 29.97 or film."
> > > 
> > > should be fine.
> > 
> > This is a bad idea IMO. Everybody calls this thing "framerate",
> > never "video rate" or anything like that.
> > Consistency is nice to have, but not at the price of
> > obfuscating things.
> 
> Whatever, that was I called av_parse_video_rate(), but that's not
> important.
> 
> If you want to go with framerate:
> char       *framerate_string; (or _str)
> AVRational  framerate;
> 
> could be an option, *or*:
> char       *framerate;
> AVRational  framerate_q;

Why? They are in different scopes so there's no conflict anywhere.
And hungarian notation isn't exactly used in libav.

--
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to