[FFmpeg-devel] [PATCH 4/8] lavu/opt: extend AVOptionRange by second value

[Michael Niedermayer]

On Tue, Feb 25, 2014 at 02:54:18AM +0100, Lukasz Marek wrote:
> On 24.02.2014 12:52, wm4 wrote:
> >On Sat, 22 Feb 2014 23:33:38 +0100
> >Lukasz Marek <lukasz.m.luki at gmail.com> wrote:
> >
> >
> >>  typedef struct AVOptionRange {
> >>      const char *str;
> >>-    double value_min, value_max;             ///< For string ranges this represents the min/max length, for dimensions this represents the min/max pixel count
> >>-    double component_min, component_max;     ///< For string this represents the unicode range for chars, 0-127 limits to ASCII
> >>-    int is_range;                            ///< if set to 1 the struct encodes a range, if set to 0 a single value
> >>+    /**
> >>+     * AV_OPT_TYPE_IMAGE_SIZE options use both elements to represent width and height ranges.
> >>+     * Other options uses only first elements.
> >>+     * For string ranges first elements represent the min/max length.
> >>+     */
> >>+    double value_min[2], value_max[2];
> >>+    /**
> >>+     * For string this represents the unicode range for chars, 0-127 limits to ASCII
> >>+     */
> >>+    double component_min, component_max;
> >>+    /**
> >>+     * If set to 1 the struct encodes a range, if set to 0 a single value
> >>+     */
> >>+    int is_range;
> >>  } AVOptionRange;
> >
> >I don't think you can do that without a major API bump. At least
> >nothing suggests that this struct is not public.
> 
> Yes, thanks I got suggested by Micheal's suggestion and didn't think
> about it. I added new fields at the end of the struct.
> 
> I will send updated patches 5-8 later. But I would be glad to not
> postpone merging 1-4 more than needed.
> 
> -- 
> Best Regards,
> Lukasz Marek
> 
> I may be drunk, Miss, but in the morning I will be sober and you
> will still be ugly. - Winston Churchill

>  opt.h |    1 +
>  1 file changed, 1 insertion(+)
> 128063d585b6006822a70b3449f72440c6e018f0  0004-lavu-opt-extend-AVOptionRange-by-extra-values.patch
> From 4a2b83198d1e79e99e8b5f40e84a9c16025bbab2 Mon Sep 17 00:00:00 2001
> From: Lukasz Marek <lukasz.m.luki at gmail.com>
> Date: Sat, 22 Feb 2014 23:32:57 +0100
> Subject: [PATCH 4/4] lavu/opt: extend AVOptionRange by extra values
> 
> TODO: micro bump?

should be minor but we could delay that if its considered unstable
and still maybe might change


> 
> AVOptionRange is not flexible enough to store AV_OPT_TYPE_IMAGE_SIZE
> ranges. Current implementation can only store pixel count.
> This patch aims to keep backward compatibility and extend
> AVOptionRange with possibility to store width/height ranges.
> 
> Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
> ---
>  libavutil/opt.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavutil/opt.h b/libavutil/opt.h
> index cd1b18e..5935e2f 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -316,6 +316,7 @@ typedef struct AVOptionRange {
>      double value_min, value_max;             ///< For string ranges this represents the min/max length, for dimensions this represents the min/max pixel count
>      double component_min, component_max;     ///< For string this represents the unicode range for chars, 0-127 limits to ASCII
>      int is_range;                            ///< if set to 1 the struct encodes a range, if set to 0 a single value
> +    double extra_value_min[2], extra_value_max[2]; ///< Dimensions use it to store min/max width and height.

this naming contradicts the existing value and component (should
be component i think)
also does any code use AVOptionRange ?
and if a compatibility layer is added whats the plan for the future
iam not sure if keeping both is a good idea and
extra_value_min seems a suboptimal name if the old is eventually
droped





>  } AVOptionRange;
>  
>  /**
> -- 
> 1.8.3.2
> 

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- 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/20140226/28e8a208/attachment.asc>