[FFmpeg-devel] [PATCHv6 4/4] libavcodec: v4l2: add support for v4l2 mem2mem codecs

[Jorge Ramirez]

On 08/28/2017 09:53 PM, wm4 wrote:
> On Mon, 28 Aug 2017 21:24:26 +0200
> Jorge Ramirez <jorge.ramirez-ortiz at linaro.org> wrote:
>
>> On 08/28/2017 02:16 PM, Jorge Ramirez wrote:
>>> On 08/28/2017 12:47 PM, wm4 wrote:
>>>>> I guess that instead of polling for the AVBufferRef to be unreferenced,
>>>>> I can associate a sync (ie a sempahore) to each buffer, take it on
>>>>> release and post the semaphore on the AVBufferRefs being unreferenced.
>>>>> that is actually pretty clean in terms of cpu usage.
>>>> That would just freeze an API user calling avcodec_close(), when it
>>>> keeps around decoded AVFrames for later use.
>>> yes I understand, but it does avoid using the CPU to poll for the
>>> buffer release (an incremental improvement)
>>>
>>> but yes I think that the message is that even though this proposal
>>> might suffice for simple video players (my tests) is not good enough
>>> for other users requiring the decoded frame for post processing.
>>>
>>> is this a blocker to upstream or could I continue working with it
>>> flagging the encoder/decoder as EXPERIMENTAL? the current situation at
>>> least keeps video players happy.
> I'd say yes this is a blocker. We usually try to avoid committing
> half-finished code, because it often means it will be never finished.

hi, I forgot to say earlier, thanks for all the review over the past 
couple of days (it has been of much help).

on the half finished matter, the issue that I face is that the current 
code doesn't cover the use case where _all_ the processed frames have to 
be kept available indefinitely (this is why I thought that perhaps 
setting .capabilities to AV_CODEC_CAP_EXPERIMENTAL could be an option to 
upstream and get more exposure to other users;

I do plan to continue supporting v4l2 ffmpeg integration - mmaped 
filters, DRM and so on...having invested this long I do want to see this 
through; and since I can't guaranteed that some "force majeure" wont 
happen I think the sooner the code I have been working on can get 
exposure the sooner we will start seeing contributions.

Anyhow, the current code does support the typical use case of most video 
players so it would benefit a considerable amount of users.

does it have to be an all or nothing at this point or could we flag the 
v4l2 m2m as experimental codecs?

>
>>>   
>> just wondering, if the AVBufferRefs must live for ever (ie, after the
>> codecs have been closed), what do other codecs dequeuing from a limited
>> number of re-usable hardware allocated buffers do?
>> do they use the CPU allocate and copy the data from those buffers to the
>> heap?
>>
> Like I wrote before: hwaccels use AVHWFramesContext, which was made
> more or less for this situation. If you want FD support later (for
> something like zero-copy transcoding or playback), AVHWFramesContext
> will probably be mandatory anyway. But I guess it's a big change for
> someone not familiar with the codebase.

Yes I had a look and it seems not an easy change to integrate.

Still I'd like to make sure we are talking about the same requirement 
because if AVHWFramesContext works around the issue [1] , I can probably 
do the same with a few more lines of code (including the FD support for 
v4l2 which is pretty straight forward)

[1]  When:
a) there is a limited number of buffers allocated by the hardware and
b) these buffers are mapped to the process address space and
c) the application can choose to keep _all_ decoded buffers for post 
processing,

then there is no other option than copying each of the processed buffers 
to newly allocated areas in the heap (there can be no magic on this 
since the hardware buffers are always limited and have to be reused).

I had a look a AVHWFRamesContext and it seems to me  that under the 
transfer frames semantics it performs some sort of memcpy in/out 
(something I could do on every capture buffer dequeue if this is the 
requirement). I could be wrong and therefore would appreciate the 
clarification if the previous comment is incorrect.

notice that I do insist on continue using V4L2_MEMORY_MMAP (instead of 
switching to V4L2_MEMORY_USERPTR) because it is easy to export the 
buffers as DMABUFs (~30 lines of code) and then pass these in FDs (which 
could be associated to short lived AVBufferRefs for DRM)


>
> But manually "nesting" AVBufferRefs to make any underlying state
> refcounted would also work.

I think so, context release now looks like this (it raises an ERROR to 
the user) but will not lock or poll.

void avpriv_v4l2_context_release(V4L2Context* ctx)
{
     struct timespec timeout = { 0, 0};
     int i, ret;

     if (!ctx->buffers)
         return;

     timeout.tv_sec = av_gettime() / 1000000 + 10;

     /* wait until all buffers owned by the user are returned */
     for (i = 0; i < ctx->num_buffers; i++) {
         for (;;) {
             ret = sem_timedwait(&ctx->buffers[i].delete_sync, &timeout);
             if (!ret)
                 break;
             if (errno == EINTR)
                 continue;
             if (errno == ETIMEDOUT)
                 av_log(ctx->log_ctx, AV_LOG_ERROR, "AVBufferRef nbr %d 
in use, bad things might happen\n", i);
             else
                 av_log(ctx->log_ctx, AV_LOG_ERROR, "sem_wait %s\n", 
av_err2str(AVERROR(errno)));
             break;
         }
     }

     ret = ctx->ops.release_buffers(ctx);
     if (ret)
         av_log(ctx->log_ctx, AV_LOG_WARNING, "V4L2 failed to unmap the 
%s buffers\n", ctx->name);
     else
         av_log(ctx->log_ctx, AV_LOG_DEBUG, "%s buffers unmapped\n", 
ctx->name);

     av_free(ctx->buffers);
     ctx->buffers = NULL;
}


thanks,

>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel