[FFmpeg-devel] [PATCH] cmdutils: dont call read_header before listing devices

[Lukasz Marek]

On 18 December 2014 at 10:41, Michael Niedermayer <michaelni at gmx.at> wrote:
>
> On Thu, Dec 18, 2014 at 01:29:39AM +0100, Lukasz Marek wrote:
> > On 18.12.2014 01:09, Michael Niedermayer wrote:
> > >On Wed, Dec 17, 2014 at 10:59:37PM +0100, Lukasz Marek wrote:
> > >>On 15.12.2014 14:18, Michael Niedermayer wrote:
> > >>>>  cmdutils.c |    8 ++++++--
> > >>>>  1 file changed, 6 insertions(+), 2 deletions(-)
> > >>>>8d012a5193b0440717f89d920661913ef160e674
> 0001-cmdutils-dont-call-read_header-before-listing-device.patch
> > >>>> From 332bb7456c498518ea72dfdaa0e8c3e76d383f21 Mon Sep 17 00:00:00
> 2001
> > >>>>From: Lukasz Marek <lukasz.m.luki2 at gmail.com>
> > >>>>Date: Mon, 15 Dec 2014 00:31:42 +0100
> > >>>>Subject: [PATCH] cmdutils: dont call read_header before listing
> devices
> > >>>>
> > >>>>List device callback must be able to return valid list without
> opening device.
> > >>>>This callback should return input values for open function, not
> vice-versa.
> > >>>>Read header funtion is very likey to fail without proper
> configuration provided.
> > >>>
> > >>>should be ok
> > >>
> > >>I changed a patch. I wanted to avoid situation where .read_close is
> > >>called without .read_header being called before.
> > >>
> > >
> > >>  cmdutils.c |   41 +++++++++++++++++++++++++++++++++++------
> > >>  1 file changed, 35 insertions(+), 6 deletions(-)
> > >>9a93c401d795bae3545a6c6112e71abd98ac22ca
> 0001-cmdutils-dont-call-read_header-before-listing-device.patch
> > >> From 58fe020b8f1c0e809362e152febe3ad715b1c8fc Mon Sep 17 00:00:00 2001
> > >>From: Lukasz Marek <lukasz.m.luki2 at gmail.com>
> > >>Date: Mon, 15 Dec 2014 00:31:42 +0100
> > >>Subject: [PATCH] cmdutils: dont call read_header before listing devices
> > >>
> > >>List device callback must be able to return valid list without opening
> device.
> > >>This callback should return input values for open function, not
> vice-versa.
> > >>Read header funtion is very likey to fail without proper configuration
> provided.
> > >>
> > >>Signed-off-by: Lukasz Marek <lukasz.m.luki2 at gmail.com>
> > >>---
> > >>  cmdutils.c | 41 +++++++++++++++++++++++++++++++++++------
> > >>  1 file changed, 35 insertions(+), 6 deletions(-)
> > >>
> > >>diff --git a/cmdutils.c b/cmdutils.c
> > >>index 4e0a406..23a5f77 100644
> > >>--- a/cmdutils.c
> > >>+++ b/cmdutils.c
> > >>@@ -2052,7 +2052,37 @@ void *grow_array(void *array, int elem_size,
> int *size, int new_size)
> > >>  }
> > >>
> > >>  #if CONFIG_AVDEVICE
> > >>-static int print_device_sources(AVInputFormat *fmt, AVDictionary
> *opts)
> > >>+static int alloc_input_context(AVFormatContext **avctx, AVInputFormat
> *iformat)
> > >>+{
> > >>+    AVFormatContext *s = avformat_alloc_context();
> > >>+    int ret = 0;
> > >>+
> > >>+    *avctx = NULL;
> > >>+    if (!s)
> > >>+        return AVERROR(ENOMEM);
> > >>+
> > >>+    s->iformat = iformat;
> > >>+    if (s->iformat->priv_data_size > 0) {
> > >>+        s->priv_data = av_mallocz(s->iformat->priv_data_size);
> > >>+        if (!s->priv_data) {
> > >>+            ret = AVERROR(ENOMEM);
> > >>+            goto error;
> > >>+        }
> > >>+        if (s->iformat->priv_class) {
> > >>+            *(const AVClass**)s->priv_data= s->iformat->priv_class;
> > >>+            av_opt_set_defaults(s->priv_data);
> > >>+        }
> > >>+    } else
> > >>+        s->priv_data = NULL;
> > >>+
> > >>+    *avctx = s;
> > >>+    return 0;
> > >>+  error:
> > >>+    avformat_free_context(s);
> > >>+    return ret;
> > >>+}
> > >>+
> > >
> > >>+static int print_device_sources(void *pfmt, AVDictionary *opts)
> > >>  {
> > >>      int ret, i;
> > >>      AVFormatContext *dev = NULL;
> > >>@@ -2069,13 +2099,12 @@ static int print_device_sources(AVInputFormat
> *fmt, AVDictionary *opts)
> > >>          goto fail;
> > >>      }
> > >>
> > >>-    /* TODO: avformat_open_input calls read_header callback which is
> not necessary.
> > >>-             Function like avformat_alloc_output_context2 for input
> could be helpful here. */
> > >>-    av_dict_copy(&tmp_opts, opts, 0);
> > >>-    if ((ret = avformat_open_input(&dev, NULL, fmt, &tmp_opts)) < 0) {
> > >>+    if ((ret = alloc_input_context(&dev, fmt)) < 0) {
> > >
> > >this fails building due to lack of a fmt variable
> >
> > I failed at cherry-pick conflict, thanks.
> > Updated patch is attached.
> >
>
> >  cmdutils.c |   39 ++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 34 insertions(+), 5 deletions(-)
> > 688f0a3fc7e3a81743bc4ce3cc858b880c648595
> 0001-cmdutils-dont-call-read_header-before-listing-device.patch
> > From 2b9a20f97687f11eb5d1fd72db3b25e3f2703b73 Mon Sep 17 00:00:00 2001
> > From: Lukasz Marek <lukasz.m.luki2 at gmail.com>
> > Date: Mon, 15 Dec 2014 00:31:42 +0100
> > Subject: [PATCH] cmdutils: dont call read_header before listing devices
> >
> > List device callback must be able to return valid list without opening
> device.
> > This callback should return input values for open function, not
> vice-versa.
> > Read header funtion is very likey to fail without proper configuration
> provided.
> >
> > Signed-off-by: Lukasz Marek <lukasz.m.luki2 at gmail.com>
> > ---
> >  cmdutils.c | 39 ++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 34 insertions(+), 5 deletions(-)
>
> should be ok, though its a bit complex if user apps need to do this
> to get the device list
>

I can move this alloc_input_context function to libavformat, I just wonder
if that would be confusing.