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

[Michael Niedermayer]

On Sun, Mar 30, 2014 at 04:06:21PM +0200, Lukasz Marek wrote:
> On 29.03.2014 21:24, Michael Niedermayer wrote:
> >On Sat, Mar 29, 2014 at 07:06:28PM +0100, Lukasz Marek wrote:
> >>>>  opt.c |   33 ++++++++++++++++++++++++---------
> >>>>  opt.h |   13 +++++++++++--
> >>>>  2 files changed, 35 insertions(+), 11 deletions(-)
> >>>>e560f163c6f61b5819e9f382615877e0bdfe12fe  0001-lavu-opt-extend-AVOptionRange-by-extra-values.patch
> >>>> From 702508c0c97812ed72deaa133140291fe3637196 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] lavu/opt: extend AVOptionRange by extra values
> >>>>
> >>>>TODO: micro bump
> >>>>
> >>>>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.c | 33 ++++++++++++++++++++++++---------
> >>>>  libavutil/opt.h | 13 +++++++++++--
> >>>>  2 files changed, 35 insertions(+), 11 deletions(-)
> >>>>
> >>>>diff --git a/libavutil/opt.c b/libavutil/opt.c
> >>>>index 652a2dd..77d20b9 100644
> >>>>--- a/libavutil/opt.c
> >>>>+++ b/libavutil/opt.c
> >>>>@@ -1511,6 +1511,7 @@ void *av_opt_ptr(const AVClass *class, void *obj, const char *name)
> >>>>
> >>>>  int av_opt_query_ranges(AVOptionRanges **ranges_arg, void *obj, const char *key, int flags)
> >>>>  {
> >>>>+    int ret, i;
> >>>>      const AVClass *c = *(AVClass**)obj;
> >>>>      int (*callback)(AVOptionRanges **, void *obj, const char *key, int flags) = NULL;
> >>>>
> >>>>@@ -1520,7 +1521,14 @@ int av_opt_query_ranges(AVOptionRanges **ranges_arg, void *obj, const char *key,
> >>>>      if (!callback)
> >>>>          callback = av_opt_query_ranges_default;
> >>>>
> >>>>-    return callback(ranges_arg, obj, key, flags);
> >>>>+    ret = callback(ranges_arg, obj, key, flags);
> >>>>+    if (ret >= 0) {
> >>>>+        if (!(flags & AV_OPT_MULTI_COMPINENT_RANGE))
> >>>>+            ret = 1;
> >>>
> >>>>+        for (i = 0; i < ret; i++)
> >>>>+            (*ranges_arg)[i].nb_components = ret;
> >>>
> >>>doesnt this  depends on sizeof(AVOptionRanges) ?
> >>>sizeof(AVOptionRanges) is not and cannot be part of the public ABI
> >>>so iam not sure how this can be accessed by a user
> >>
> >>Yes, it does depend on sizeof(AVOptionRanges)
> >>
> >>>>  /**
> >>>>+ *  Allows av_opt_query_ranges and av_opt_query_ranges_default to return more than
> >>>>+ *  one instance of AVOptionRanges.
> >>>>+ */
> >>>>+#define AV_OPT_MULTI_COMPINENT_RANGE 0x0004
> >>>
> >>>what is a compInent ?
> >>>also how and where were the ranges defined for multi component options
> >>>before?
> >>>If there where not defined at all then a change is possibly not a
> >>>API/ABI break and this can be simplified away
> >>
> >>It is not documented yet.
> >>
> >>>also what do multiple component ranges mean exactly?
> >>>range1: 320,640
> >>>range2: 240,480
> >>>
> >>>means 320x240 || 640x480
> >>>or
> >>>means 320x240 || 320x480 || 640x240 || 640x480
> >>>(this should be documented, and one of these looks insufficient)
> >>
> >>I have first option in mind. Each AVOptionRanges must have the same
> >>value on nb_ranges. So valid ranges are:
> >>
> >>{ AVOptionRanges[0].range[0], AVOptionRanges[1].range[0], ... },
> >>{ AVOptionRanges[0].range[1], AVOptionRanges[1].range[1], ... },
> >>...
> >>
> >>Please elaborate, because it don't know what case you have in mind
> >>this time.
> >
> >this one should work, yes
> >
> >
> >
> >>
> >>>also you can put any N-Dimensional array in a 1d array like:
> >>>ranges[component*nb_ranges + range] or
> >>>ranges[componen + range*nb_components]
> >>
> >>This may help to get rid of sizeof(AVOptionRanges) dependency,
> >>thanks for hint.
> >>
> >>I will fix it somehow, but please answer for question above.
> 
> Changed patch attached. It is very similar to previous, but stores
> all components in one array as you hinted.
> I haven't reimplemented opengl to test it, but I suppose there
> should be no problems with it.

looks fine, maybe wait a day or 2 though so others can comment too
its sure possible that iam missing something

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Avoid a single point of failure, be that a person or equipment.
-------------- 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/20140330/bab31146/attachment.asc>