[FFmpeg-devel] [PATCH] libavformat: Add FIFO pseudo-muxer

Jan Sebechlebsky sebechlebskyjan at gmail.com
Mon Jul 4 17:37:15 EEST 2016


Hello Nicolas,
I am sending the second version of patch, I have some notes why certain 
things from your feedback are not implemented (yet?) and additional 
questions :)

On 06/28/2016 03:42 PM, Nicolas George wrote:
>
>> + at item block_on_overflow 0|1
>>
>> +If set to 0, fully non-blocking mode will be used and in case the queue
>> +fills up, packets will be dropped. By default this option is set to 1,
>> +so in case of queue overflow the encoder will be block until muxer
>> +processes some of the packets.
> IMHO, this should use AVFMT_FLAG_NONBLOCK.
>
This is not yet exposed to the user via cmd options, do you think I 
should add this flag to options in option_table.h (for encoding only)?
>
> I had to look twice to be sure it was only used in the thread.
>
> Suggestions:
>
> - make sure function names describe exactly where they can be called:
>    fifo_thread_description() for functions called in the thread,
>    fifo_description() for functions called from the main thread,
>    description() for utility functions that are used in both but do nothing
>    dangerous.
>
> - move fields that are only useful in the thread to a different struct,
>    allocate it on the stack in the main function of the thread;
>
> - try to make the FifoContext pointer const in the thread.
I've renamed the functions and also moved the fields accessed only from 
the thread to separate structure, however I didn't make the FifoContext 
const in thread - it would require creating another structure for the 
fields accessed both by "main" thread and fifo thread (or some other 
solution which seemed more messy to me).

>
>> +    avf2->interrupt_callback = avf->interrupt_callback;
> This is wrong. First, we do not know that the application-provided callback
> is thread-safe. Second, the thread needs an interrupt_callback of its own to
> interrupt the actual I/O when asked to abort.
Can you please explain little bit more why is this wrong (appart from 
the undocumented requirement for the interrupt callback to be thread-safe)?
>> +    FifoMessage message = {.type = FIFO_WRITE_HEADER};
>> +
>> +    ret = av_thread_message_queue_send(fifo->queue, &message, 0);
>> +    if (ret < 0)
>> +        return ret;
> This message is sent only once. Maybe take it out of the loop to make things
> simpler.
I decided not to take it out of the loop - it would be a special case to 
handle in case the call would fail.

Regards,
Jan


More information about the ffmpeg-devel mailing list