From: Mauro Carvalho Chehab <mchehab@redhat.com>
To: Kamil Debski <k.debski@samsung.com>
Cc: "'Sakari Ailus'" <sakari.ailus@iki.fi>,
linux-media@vger.kernel.org,
"'Laurent Pinchart'" <laurent.pinchart@ideasonboard.com>,
"'Sebastian Dröge'" <sebastian.droege@collabora.co.uk>,
"Sylwester Nawrocki" <s.nawrocki@samsung.com>,
"Marek Szyprowski" <m.szyprowski@samsung.com>
Subject: Re: [RFC] Resolution change support in video codecs in v4l2
Date: Fri, 02 Dec 2011 16:09:54 -0200 [thread overview]
Message-ID: <4ED91472.9000907@redhat.com> (raw)
In-Reply-To: <007201ccb118$633ff890$29bfe9b0$%debski@samsung.com>
On 02-12-2011 15:32, Kamil Debski wrote:
>>> Usually there is a minimum number of buffers that has to be kept for future
>>> references. New frames reference previous frames (and sometimes the following
>>> frames as well) to achieve better compression. If we haven't got enough buffers
>>> decoding cannot be done.
>>
>> OK, but changing the resolution won't affect the number of buffers needed
>> for
>> inter-frame interpolation.
>
> Changing resolution alone will not affect it, but changing the resolution
> is very often associated with other changes in the stream. Imagine that there
> are two streams - one 720P that needs 4 buffer and second 1080P that needs 6.
> When the change is done - both resolution is increased and the minimum
> number of buffers.
OK. In this case, if just 4 buffers were pre-allocated, driver will need to
stop streaming (or the application should start with 6 buffers, if it needs
to support such changes without stopping the stream).
>> As I said before, a dqueued buffer is assomed to be a buffer where the
>> Kernel
>> won't use it anymore. If kernel still needs it, just don't dequeue it yet.
>> Anything different than that may cause memory corruption, cache coherency
>> issues, etc.
>
> I agree. This flag could be used as a hint when the display delay is enforced.
> On the other hand - when the application requests an arbitrary display delay
> then it should be aware that modifying those buffers is risky.
Looking at the issue from the other side, I can't see what the application would
do with such hint.
> The display delay is the number of buffers/frames that the codec processes before
> returning the first buffer. For example if it is set to 0 then it returns the
> buffers ASAP, not regarding their order or that they are still used.
> This functionality can be used to create thumbnail images of movies in a gallery
> (by decoding a single frame from the beginning of the movie).
A 0-delay buffer like the above described would probably mean that the userspace
application won't touch on it anyway, as it needs that buffer as soon as possible.
A single notice at the API spec should be enough to cover this case.
>>>>> The minimal number of buffers is more related to latency issues and
>>>> processing
>>>>> speed at userspace than to any driver or format-dependent hardware
>>>> constraints.
>>>>>
>>>>> On the other hand, the maximum number of buffers might eventually have
>>>> some
>>>>> constraint, e. g. a hardware might support less buffers, if the
>> resolution
>>>>> is too high.
>>>>>
>>>>> I prefer to not add anything to the V4L2 API with regards to changes at
>>>> max/min
>>>>> number of buffers, except if we actually have any limitation at the
>>>> supported
>>>>> hardware. In that case, it will likely need a separate flag, to indicate
>>>> userspace
>>>>> that buffer constraints have changed, and that audio buffers will also
>>>> need to be
>>>>> re-negotiated, in order to preserve A/V synch.
>>>>
>>>> I think that boils down to the properties of the codec and possibly also
>> the
>>>> stream.
>>>
>>> If a timestamp or sequence number is used then I don't see why should we
>>> renegotiate audio buffers. Am I wrong? Number of audio and video of
>> buffers
>>> does not need to be correlated.
>>
>> Currently, alsa doesn't put a timestamp on the buffers. Ok, this is
>> something
>> that require fixes there.
>
> Ups, I did not know this. If so then there we should think about a method to
> synchronize audio and video.
Yes. Someone needs to come up with some patches for alsa. This seems to be
the right thing to do.
>>>>>> 5) After the application does STREMON the processing should continue.
>> Old
>>>>>> buffers can still be used by the application (as CREATE_BUFS was used),
>>>> but
>>>>>> they should not be queued (error shall be returned in this case). After
>>>> the
>>>>>> application is finished with the old buffers it should free them with
>>>>>> DESTROY_BUFS.
>>>>>
>>>>> If the buffers are bigger, there's no issue on not allowing queuing them.
>>>> Enforcing
>>>>> it will likely break drivers and eventually applications.
>>>>
>>>> I think this means buffers that are too small for the new format. They
>> are
>>>> no longer needed after they have been displayed --- remember there must
>> also
>>>> be no interruption in displaying the video.
>>>
>>> Yes, I have meant the buffers that are too small.
>>>
>
>
> Best wishes,
Regards,
Mauro.
next prev parent reply other threads:[~2011-12-02 18:10 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-02 10:31 [RFC] Resolution change support in video codecs in v4l2 Kamil Debski
2011-12-02 12:35 ` Mauro Carvalho Chehab
2011-12-02 13:57 ` Sakari Ailus
2011-12-02 15:41 ` Kamil Debski
2011-12-02 17:07 ` Mauro Carvalho Chehab
2011-12-02 17:32 ` Kamil Debski
2011-12-02 18:09 ` Mauro Carvalho Chehab [this message]
2011-12-06 12:00 ` Laurent Pinchart
2011-12-06 14:28 ` 'Sakari Ailus'
2011-12-06 14:41 ` Mauro Carvalho Chehab
2011-12-06 15:19 ` Kamil Debski
2011-12-06 15:40 ` Mauro Carvalho Chehab
2011-12-06 16:11 ` Kamil Debski
2011-12-06 16:39 ` Mauro Carvalho Chehab
2011-12-07 11:49 ` Kamil Debski
2011-12-10 9:17 ` 'Sakari Ailus'
2011-12-12 11:11 ` Laurent Pinchart
2011-12-03 0:08 ` 'Sakari Ailus'
2011-12-05 13:01 ` Mauro Carvalho Chehab
2011-12-02 16:50 ` Mauro Carvalho Chehab
2011-12-06 14:35 ` Sakari Ailus
2011-12-06 15:03 ` Kamil Debski
2011-12-09 19:54 ` 'Sakari Ailus'
2011-12-12 10:17 ` Kamil Debski
2012-01-01 22:29 ` 'Sakari Ailus'
2012-01-04 10:19 ` Kamil Debski
2012-01-11 22:30 ` 'Sakari Ailus'
2011-12-12 10:59 ` Laurent Pinchart
2011-12-12 11:09 ` Kamil Debski
2011-12-06 16:20 ` Mauro Carvalho Chehab
2011-12-06 22:41 ` Sakari Ailus
2011-12-07 11:12 ` Kamil Debski
2011-12-09 19:58 ` 'Sakari Ailus'
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4ED91472.9000907@redhat.com \
--to=mchehab@redhat.com \
--cc=k.debski@samsung.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=s.nawrocki@samsung.com \
--cc=sakari.ailus@iki.fi \
--cc=sebastian.droege@collabora.co.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.