[vlc-devel] [PATCH 3/3] mediacodec: implementation of MediaCodec direct rendering based on the work by Martin Storsjö.

[Felix Abecassis]

2013/12/13 Martin Storsjö <martin at martin.st>:
>
> I guess that's a pretty bold assumption. Yes, some of the preliminaries have
> been fixed now, but did this patch get a proper review by someone who knows
> the internals properly?
There have been some discussions going on in the cover letter of this
patch serie ([PATCH 0/3] Android MediaCodec direct rendering) and it
lead to the removing of decoder buffering in the input code. I'm still
open to other opinions on this patch but I think people had time to
review it.
Even for those who think they don't have the authority to comment on
the design choices, it would be great if more people could test this
patch on their devices. You will need the above patch and this one for
vlc-android: https://github.com/mstorsjo/vlc-android/commit/43a8a1c217717e5dba627dea46a6900c72d43244

> In general this seems to work pretty well on my Galaxy S3 now - kudos for
> that. However, if pausing the video, waiting for a little while (a few
> seconds) and then pressing the back key to exit, something somewhere seems
> to deadlock.
Which sample did you use?

2013/12/13 Martin Storsjö <martin at martin.st>:
> Why do you need to explicitly check for this particular exception class?
In order to be consistent with the other call to releaseOutputBuffer
in the non-DR path.
But you are right, the correct solution would be to remove the
explicit checks after both calls.

> nit: While VLC generally has favoured this indentation style, it's not the
> general norm all over the place. Most of this file places the braces on the
> same line as the if/else, so it would be more consistent with the rest of
> the file in that form.
Ah, yes indeed. I will fix that.

> This is a little bit hackish - having one plugin define a public symbol that
> other plugins directly link against. Yes, it works on android right now when
> all the plugins are statically linked into one big blob, but it violates
> general VLC plugin designs. (No, I don't have other suggestions either.)
I considered allocating the mutex in the decoder structure and pass it
in the p_sys structure. However since the decoder module can be
closed/reopened while some pictures are still in flight, we wouldn't
know when to free the mutex.

I think a similar approach is used in the VDPAU code in order to
manage the VDP instance. Correct me if I'm wrong, Rémi.


-- 
Félix Abecassis
http://felix.abecassis.me