Re: [FFmpeg-devel] [PATCH v1 1/3] avcodec: v4l2_m2m: fix races around freeing data on close

[Jorge Ramirez-Ortiz]

On 01/21/2018 01:46 AM, Mark Thompson wrote:

On 09/01/18 22:56, Jorge Ramirez-Ortiz wrote:

From: Mark Thompson 

Refcount all of the context information. This also fixes a potential
segmentation fault when accessing freed memory  (buffer returned after
the codec has been closed).

Tested-by: Jorge Ramirez-Ortiz 
---
  libavcodec/v4l2_buffers.c | 32 ++--
  libavcodec/v4l2_buffers.h |  6 +++
  libavcodec/v4l2_m2m.c | 93 +--
  libavcodec/v4l2_m2m.h | 35 ++
  libavcodec/v4l2_m2m_dec.c | 22 +++
  libavcodec/v4l2_m2m_enc.c | 22 +++
  6 files changed, 140 insertions(+), 70 deletions(-)

I've done some more testing of this set on s5p-mfc.  I still don't like the 
residual use of atomics in what I wrote, but it is overall a bit better than 
before so I've applied the whole set.

Other stuff:
* There's a timestamp overflow case, patch follows.


ok

* Reinitialisation still fails in some cases - decoding 
fate/h264/reinit-large_420_8-to-small_420_8.h264 currently hangs.  That needs 
more investigation, which I haven't done.


yes, that needs to be done properly: I think what was merged at the time 
was working around some v4l2 kernel issues in the db820c so it wasnt 
quite following the expected kernel API hence why it might not work on 
other boards.




* valgrind still shows leaks when h264_mp4toannexb feeds the decoder, but the 
bsf doesn't show it in isolation.  Buffer references leaking in the decoder 
somehow?


you suspect it to be in the v4l2 decoder or in the framework? I did a 
backport to 3.3 some time ago (so not using h264_mp4toannexb as you also 
mentioned) and confirmed that the leaks disappeared. Since the overall 
decoder didn't change it thought it had to be something broken in the 
framework.




Thanks,

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v1 1/3] avcodec: v4l2_m2m: fix races around freeing data on close

[Jorge Ramirez-Ortiz]

On 01/22/2018 12:25 AM, Mark Thompson wrote:

On 19/01/18 16:40, Jorge Ramirez-Ortiz wrote:

On 01/19/2018 12:30 AM, Michael Niedermayer wrote:

On Thu, Jan 18, 2018 at 09:24:20AM +0100, Jorge Ramirez-Ortiz wrote:

On 01/09/2018 11:56 PM, Jorge Ramirez-Ortiz wrote:

From: Mark Thompson 

Refcount all of the context information. This also fixes a potential
segmentation fault when accessing freed memory  (buffer returned after
the codec has been closed).

just a follow up on the patchset (patches 1 to 3)
any feedback? shall I resend?

Who is the maintainer of this code ?
noone is listed for it MAINTAINERS

if someone volunteers to maintain it and there are no objections
then that person would get git write access and could push patches
and then bugfixes wont be stuck so long ...

If someone wants to volunteer in that sense, then please send a patch
for the MAINTAINER file

sure I can send such a patch ( unless Mark Thompson has an interest in 
maintaining it -since he did most of the reviews of the whole patchset IIRC- 
Baylibre will give me time to maintain this code)

I only have peripheral interest in the v4l2 stuff and only one board to test 
on, so I don't really feel up to maintaining it - if you're happy to be 
responsible for it then please go ahead.


ok


  (I'm still available to look at patches and test stuff as I do now if that's 
helpful.)


yes please - at the moment I only have the db820c whose v4l2 kernel 
support is still being improved. I'll order another board with more 
stable v4l2 support for this activity so I can complete the 
restart/resolution changes.


thanks!



Thanks,

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v1 1/3] avcodec: v4l2_m2m: fix races around freeing data on close

[Mark Thompson]

On 19/01/18 16:40, Jorge Ramirez-Ortiz wrote:
> On 01/19/2018 12:30 AM, Michael Niedermayer wrote:
>> On Thu, Jan 18, 2018 at 09:24:20AM +0100, Jorge Ramirez-Ortiz wrote:
>>> On 01/09/2018 11:56 PM, Jorge Ramirez-Ortiz wrote:
 From: Mark Thompson 

 Refcount all of the context information. This also fixes a potential
 segmentation fault when accessing freed memory  (buffer returned after
 the codec has been closed).
>>> just a follow up on the patchset (patches 1 to 3)
>>> any feedback? shall I resend?
>> Who is the maintainer of this code ?
>> noone is listed for it MAINTAINERS
>>
>> if someone volunteers to maintain it and there are no objections
>> then that person would get git write access and could push patches
>> and then bugfixes wont be stuck so long ...
>>
>> If someone wants to volunteer in that sense, then please send a patch
>> for the MAINTAINER file
> 
> sure I can send such a patch ( unless Mark Thompson has an interest in 
> maintaining it -since he did most of the reviews of the whole patchset IIRC- 
> Baylibre will give me time to maintain this code)

I only have peripheral interest in the v4l2 stuff and only one board to test 
on, so I don't really feel up to maintaining it - if you're happy to be 
responsible for it then please go ahead.  (I'm still available to look at 
patches and test stuff as I do now if that's helpful.)

Thanks,

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v1 1/3] avcodec: v4l2_m2m: fix races around freeing data on close

[Mark Thompson]

On 09/01/18 22:56, Jorge Ramirez-Ortiz wrote:
> From: Mark Thompson 
> 
> Refcount all of the context information. This also fixes a potential
> segmentation fault when accessing freed memory  (buffer returned after
> the codec has been closed).
> 
> Tested-by: Jorge Ramirez-Ortiz 
> ---
>  libavcodec/v4l2_buffers.c | 32 ++--
>  libavcodec/v4l2_buffers.h |  6 +++
>  libavcodec/v4l2_m2m.c | 93 
> +--
>  libavcodec/v4l2_m2m.h | 35 ++
>  libavcodec/v4l2_m2m_dec.c | 22 +++
>  libavcodec/v4l2_m2m_enc.c | 22 +++
>  6 files changed, 140 insertions(+), 70 deletions(-)

I've done some more testing of this set on s5p-mfc.  I still don't like the 
residual use of atomics in what I wrote, but it is overall a bit better than 
before so I've applied the whole set.

Other stuff:
* There's a timestamp overflow case, patch follows.
* Reinitialisation still fails in some cases - decoding 
fate/h264/reinit-large_420_8-to-small_420_8.h264 currently hangs.  That needs 
more investigation, which I haven't done.
* valgrind still shows leaks when h264_mp4toannexb feeds the decoder, but the 
bsf doesn't show it in isolation.  Buffer references leaking in the decoder 
somehow?

Thanks,

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v1 1/3] avcodec: v4l2_m2m: fix races around freeing data on close

[Jorge Ramirez-Ortiz]

On 01/19/2018 12:30 AM, Michael Niedermayer wrote:

On Thu, Jan 18, 2018 at 09:24:20AM +0100, Jorge Ramirez-Ortiz wrote:

On 01/09/2018 11:56 PM, Jorge Ramirez-Ortiz wrote:

From: Mark Thompson 

Refcount all of the context information. This also fixes a potential
segmentation fault when accessing freed memory  (buffer returned after
the codec has been closed).

just a follow up on the patchset (patches 1 to 3)
any feedback? shall I resend?

Who is the maintainer of this code ?
noone is listed for it MAINTAINERS

if someone volunteers to maintain it and there are no objections
then that person would get git write access and could push patches
and then bugfixes wont be stuck so long ...

If someone wants to volunteer in that sense, then please send a patch
for the MAINTAINER file


sure I can send such a patch ( unless Mark Thompson has an interest in 
maintaining it -since he did most of the reviews of the whole patchset 
IIRC- Baylibre will give me time to maintain this code)





[...]


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v1 1/3] avcodec: v4l2_m2m: fix races around freeing data on close

[Michael Niedermayer]

On Thu, Jan 18, 2018 at 09:24:20AM +0100, Jorge Ramirez-Ortiz wrote:
> On 01/09/2018 11:56 PM, Jorge Ramirez-Ortiz wrote:
> >From: Mark Thompson 
> >
> >Refcount all of the context information. This also fixes a potential
> >segmentation fault when accessing freed memory  (buffer returned after
> >the codec has been closed).
> 
> just a follow up on the patchset (patches 1 to 3)
> any feedback? shall I resend?

Who is the maintainer of this code ?
noone is listed for it MAINTAINERS

if someone volunteers to maintain it and there are no objections
then that person would get git write access and could push patches
and then bugfixes wont be stuck so long ...

If someone wants to volunteer in that sense, then please send a patch
for the MAINTAINER file


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v1 1/3] avcodec: v4l2_m2m: fix races around freeing data on close

[Jorge Ramirez-Ortiz]

On 01/09/2018 11:56 PM, Jorge Ramirez-Ortiz wrote:

From: Mark Thompson 

Refcount all of the context information. This also fixes a potential
segmentation fault when accessing freed memory  (buffer returned after
the codec has been closed).


just a follow up on the patchset (patches 1 to 3)
any feedback? shall I resend?

thanks!


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel