[vlc-devel] nvdec GPU to CPU converter

[Quentin Chateau]

On 03/03/2020 08:30, Steve Lhomme wrote:
> Hi Quentin,
>
> On 2020-03-02 18:56, Quentin Chateau wrote:
>> Hi,
>>
>> You recently fixed vout_DisableWindow to not release the decoder 
>> device on failure. Thanks to this patch, the NVDEC decoder can now 
>> correctly 
>
> You're the one who did all the work ;)
>
>> fallback to a non-opaque chroma (CPU buffer) if the video 
>> initialization using an opaque chroma fails.
>>
>>      for (chroma_idx = 0; output_chromas[chroma_idx] != 0; chroma_idx++)
>>      {
>>          p_dec->fmt_out.i_codec = p_dec->fmt_out.video.i_chroma = 
>> output_chromas[chroma_idx];
>>          result = decoder_UpdateVideoOutput(p_dec, p_sys->vctx_out);
>>          if (result == VLC_SUCCESS)
>>          {
>>              msg_Dbg(p_dec, "using chroma %4.4s", 
>> (char*)&p_dec->fmt_out.video.i_chroma);
>>              break;
>>          }
>>          msg_Warn(p_dec, "Failed to use output chroma %4.4s", 
>> (char*)&p_dec->fmt_out.video.i_chroma);
>>      }
>>
>> Previously, only the first chroma of the output_chromas array could 
>> succeed due to the vout_DisableWindow bug.
>>
>> It seems to me the GPU to CPU video filter (chroma.c file in the same 
>> directory as nvdec.c) is now useless: the decoder can already output 
>> either CPU or GPU buffers. Worse than than, its very existence 
>> degrades performance in the case where a CPU buffer is used as output.
>>
>> Currently, even if the pipeline requires a CPU buffer as output, the 
>> decoder will output GPU buffers and the GPU-to-CPU video filter will 
>> be used: 1 GPU to GPU (decoder) + 1 GPU to CPU (filter) copies are made.
>>
>> Disabling the video filter, the behavior becomes: the first call to 
>> decoder_UpdateVideoOutput fails, then the decoder retries with a 
>> non-opaque chroma and succeeds. In this configuration, only 1 GPU to 
>> CPU copy is performed by the decoder. The pipeline performance is 
>> objectively better (1 less GPU-GPU copy), but a lot of error logs are 
>> printed out during the first attempt by the decoder to use opaque 
>> chromas.
>>
>>  1. Do you see any reason to keep the GPU-to-CPU video filter ?
>
> Yes. Even if the initial playback is using the GPU output, the user 
> may add a CPU based filter during playback and a conversion will be 
> required. The decoder will know nothing about that and will not change 
> its output.
Oh indeed, I did not think about changes during playback.
>
>>  2. I think a --nvdec-prefer-opaque/--no-nvdec-prefer-opaque could be a
>>     good addition if one knows in advance which chroma type will be
>>     used. This would avoid the wall of error logs in certain
>>     circumstances. What do you think ?
>
> The software output from the decoder only exists as a convenience. In 
> an ideal world the decoder should only output NVDEC chromas and the 
> conversion be done elsewhere. Maybe we can remove that part now that 
> the push model is (mostly) done.

I also agree the CPU fallback of the decoder can be removed. To be fair, 
it will probably never get used, since the GPU/CPU filter will always be 
used instead. Add to that, due to the vout_DisableWindow bug, I think it 
could never be used anyway.

Not sure where you guys stand on that, the only drawback is the extra 
GPU/GPU copy compared to a direct CPU output. Although looking at the 
code, we may even be able to get rid of that.

In any case, I think that we either want to remove the CPU fallback, or 
make it usable (for instance though a parameter as I suggested)

>
> One of the issue to take care of is the CPU fallback doesn't use I420 
> first as it will find converters to do that, but with extra filters 
> and copies. For most hardware decoders the best fallback to try is 
> NV12 (and P010/P016). I don't think the core is ready for that yet. 
> Same thing with 444 output.
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel