[FFmpeg-devel] [PATCH] libavdevice: JACK demuxer

Olivier Guilyardi list
Wed Mar 18 16:22:18 CET 2009


Michael Niedermayer wrote:
> On Fri, Mar 13, 2009 at 09:05:38PM +0100, Olivier Guilyardi wrote:
[...]
> 
>> +#define FIFO_WRITE_SPACE(fifo) (fifo->end - fifo->buffer - av_fifo_size(fifo))
> 
> i would prefer if you did not use end/buffer directly but kept a private
> variable that is the size

Okay, I'll do that.

> [...]
>> +        if (av_new_packet(&pkt, pkt_size) < 0) {
>> +            av_log(context, AV_LOG_ERROR, "Could not create packet of size %d\n", pkt_size);
>> +            return AVERROR(EIO);
>> +        }
>> +        av_fifo_generic_write(self->new_pkt_fifo, &pkt, sizeof(pkt), NULL);
> 
> it should be possible to use the function ptr argument of
> av_fifo_generic_write to avoid the copy

Is saving the copy of 60 bytes worth the cost of:
- extra function calls (that involves stack manipulation and others)
- additional function and source lines
- losing the readability of this simple data writing operation
- spending your and my precious time on this
?

> [...]
>> +static int audio_read_packet(AVFormatContext *context, AVPacket *pkt)
>> +{
>> +    JackData *self = context->priv_data;
>> +    struct timespec timeout = {0, 0};
>> +    int test;
>> +
>> +    /* Activate the jack client on first packet read. Activating the JACK client
>> +     * means that process_callback() starts to get called at regular interval.
>> +     * If we activate it in audio_read_header(), we're actually reading audio data
>> +     * from the device before instructed to, and that may result in an overrun. */
>> +    if (!self->activated && activate_jack(self, context))
>> +        return AVERROR(EIO);
> 
> i still think this belongs in audio_read_header()

As an application writer, it looks to me like you might want to read the header
of the input streams you handle, then wait an undetermined amount of time before
starting to read the packets. For example, when you first check that the streams
are ok, look at their characteristics, inform the user, and then wait him/her to
press "Play/Record".

If a long time elapses between audio_read_header() and audio_read_packet(), and
if activate_jack() is called in the former, the following is going is to happen:
1 - the ringbuffer starts to get filled with data captured at t + 0
2 - then we wait an undetermined time until t + N
3 - the application starts to read packets for real

In 2 we're likely to get an overrun. But the true problem is that on the first
calls, audio_read_packet() will return a few packets captured from t + 0. That
will free some space in the ringbuffer, and the next packets returned will be
the ones captured from t + N.

In conclusion, you'll get audio data that include a few packets of "past" at the
beginning.

--
  Olivier





More information about the ffmpeg-devel mailing list