Re: [libav-devel] [PATCH 1/2] libfdk-aacdec: Enable Decoder Downmix including Downmix Metadata Support
On Fri, 17 Oct 2014, Tim Walker wrote: On 15 Oct 2014, at 13:40, Martin Storsjö wrote: When downmixing multichannel streams, the decoder requires the output buffer in aacDecoder_DecodeFrame call to be of fixed size in order to hold the actual number of channels contained in the stream. For example, for a 5.1ch to stereo downmix, the decoder requires that the output buffer is allocated for 6 channels, regardless of the fact that the output is in fact two channels. Due to this requirement, the output buffer is allocated for the maximum output buffer size in case a downmix is requested (and also during decoder init). When a downmix is requested, the buffer used for output during init will also be used for the entire duration the decoder is open. Otherwise, the initial decoder output buffer is freed and a buffer large enough only for the actual output size is used instead. I'm having trouble following. Let's say we have a 5.1 source: - during init, we allocate a buffer than can hold 6 channels' worth of decoded data Actually, this buffer only is allocated on the first decode_frame call, not during init, and for non-downmix cases we free the temp buffer when the first frame has been decoded successfully, but that's details - if a downmix is requested, the 6-channel buffer allocated during init gets used - if no downmix is requested, the 6-channel buffer allocated during init is freed and a buffer large enough only for the actual output size is used, but, that would also be a 6-channel buffer, so what's the point of not using the initial buffer? It's not "a buffer large enough only for the actual output size", it's "the output AVFrame" that is allocated once we know the exact number of channels, vs a generic buffer allocated with av_malloc. The point in not using the initial buffer is that we'd rather decode straight into the AVFrame if possible, to avoid an extra memcpy. I'm not sure if it's kosher to request e.g. a 6 channel AVFrame and then just flip it to 2 channels after allocation - if it is, then we could of course simplify that, but that is unrelated to this patch, this is what we do already. Does the commit message fail to mention we may have a "downmix" scenario where the maximum number of channels is greater than the actual number of channels (aka "upmix")? I tested this, and the library won't upmix in this case Does the commit message fail to mention another scenario not related to the number of channels? I don't know? // Martin ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 1/2] libfdk-aacdec: Enable Decoder Downmix including Downmix Metadata Support
On 15 Oct 2014, at 13:40, Martin Storsjö wrote: > When downmixing multichannel streams, the decoder requires the output > buffer in aacDecoder_DecodeFrame call to be of fixed size in order to > hold the actual number of channels contained in the stream. For example, > for a 5.1ch to stereo downmix, the decoder requires that the output buffer > is allocated for 6 channels, regardless of the fact that the output is in > fact two channels. > > Due to this requirement, the output buffer is allocated for the maximum > output buffer size in case a downmix is requested (and also during > decoder init). When a downmix is requested, the buffer used for output > during init will also be used for the entire duration the decoder is open. > Otherwise, the initial decoder output buffer is freed and a buffer large > enough only for the actual output size is used instead. I'm having trouble following. Let's say we have a 5.1 source: - during init, we allocate a buffer than can hold 6 channels' worth of decoded data - if a downmix is requested, the 6-channel buffer allocated during init gets used - if no downmix is requested, the 6-channel buffer allocated during init is freed and a buffer large enough only for the actual output size is used, but, that would also be a 6-channel buffer, so what's the point of not using the initial buffer? Does the commit message fail to mention we may have a "downmix" scenario where the maximum number of channels is greater than the actual number of channels (aka "upmix")? Does the commit message fail to mention another scenario not related to the number of channels? Tim ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 2/3] rtmpproto: Add getStreamLength call to query duration
On Thu, 16 Oct 2014, Uwe L. Korn wrote: On 15/10/14 12:06, Martin Storsjö wrote: On Tue, 14 Oct 2014, Uwe L. Korn wrote: In (non-live) streams with no metadata, the duration of a stream can I assume it doesn't hurt in case this is sent for a live stream, i.e. it's ignored and/or returns 0 in those cases as well? I had no live stream to test with, but with invalid playpaths, I always got a duration of 0 replied from the server. Not sure if different implementations will answer something else but I don't expect any server to reply with a positive number where the stream has no duration. But sending it for a live stream just sounds like unnecessary overhead as we already know that we'll get no valid duration and thus just spent a bit more time for initiating the stream which we do not need. My point is, you send it if rt->live != -1; where -1 corresponds to "live", but the default is -2, and for _most_ live streams I've seen, you don't need to set it (the default works fine). So for the case when you connect to a rtmp url not knowing what it is, having rt->live set to the default, the getStreamLength packet will be sent, which shouldn't hopefully do any major harm if it turns out it actually was a live stream. // Martin ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH 2/3] rtmpproto: Add getStreamLength call to query duration
On 15/10/14 12:06, Martin Storsjö wrote:
> On Tue, 14 Oct 2014, Uwe L. Korn wrote:
>
>> In (non-live) streams with no metadata, the duration of a stream can
>
> I assume it doesn't hurt in case this is sent for a live stream, i.e.
> it's ignored and/or returns 0 in those cases as well?
>
I had no live stream to test with, but with invalid playpaths, I always
got a duration of 0 replied from the server. Not sure if different
implementations will answer something else but I don't expect any server
to reply with a positive number where the stream has no duration.
But sending it for a live stream just sounds like unnecessary overhead
as we already know that we'll get no valid duration and thus just spent
a bit more time for initiating the stream which we do not need.
>> be retrieved by calling the RTMP function getStreamLength with the
>> playpath. The server will return a positive duration upon the request if
>> the duration is known, otherwise either no response or a duration of 0
>> will be returned.
>> ---
>> libavformat/rtmpproto.c | 106
>>
>> 1 file changed, 106 insertions(+)
>>
>> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
>> index 2b4c173..6e90a9f 100644
>> --- a/libavformat/rtmpproto.c
>> +++ b/libavformat/rtmpproto.c
>> @@ -123,6 +123,7 @@ typedef struct RTMPContext {
>> int listen; ///< listen mode flag
>> int listen_timeout; ///< listen timeout to
>> wait for new connections
>> int nb_streamid;///< The next stream id
>> to return on createStream calls
>> +doubleduration; ///< Duration of the
>> stream in seconds as returned by the server (only valid if non-zero)
>> char username[50];
>> char password[50];
>> char auth_params[500];
>> @@ -679,6 +680,30 @@ static int gen_delete_stream(URLContext *s,
>> RTMPContext *rt)
>> }
>>
>> /**
>> + * Generate 'getStreamLength' call and send it to the server. If the
>> server
>> + * knows the duration of the selected stream, it will reply with the
>> duration
>> + * in seconds.
>> + */
>> +static int gen_get_stream_length(URLContext *s, RTMPContext *rt)
>> +{
>> +RTMPPacket pkt;
>> +uint8_t *p;
>> +int ret;
>> +
>> +if ((ret = ff_rtmp_packet_create(&pkt, RTMP_SOURCE_CHANNEL,
>> RTMP_PT_INVOKE,
>> + 0, 31 + strlen(rt->playpath))) < 0)
>> +return ret;
>> +
>> +p = pkt.data;
>> +ff_amf_write_string(&p, "getStreamLength");
>> +ff_amf_write_number(&p, ++rt->nb_invokes);
>> +ff_amf_write_null(&p);
>> +ff_amf_write_string(&p, rt->playpath);
>> +
>> +return rtmp_send_packet(rt, &pkt, 1);
>> +}
>> +
>> +/**
>> * Generate client buffer time and send it to the server.
>> */
>> static int gen_buffer_time(URLContext *s, RTMPContext *rt)
>> @@ -2044,11 +2069,19 @@ static int handle_invoke_result(URLContext *s,
>> RTMPPacket *pkt)
>> if ((ret = gen_publish(s, rt)) < 0)
>> goto fail;
>> } else {
>> +if (rt->live != -1) {
>> +if ((ret = gen_get_stream_length(s, rt)) < 0)
>> +goto fail;
>> +}
>> if ((ret = gen_play(s, rt)) < 0)
>> goto fail;
>> if ((ret = gen_buffer_time(s, rt)) < 0)
>> goto fail;
>> }
>> +} else if (!strcmp(tracked_method, "getStreamLength")) {
>> +if (read_number_result(pkt, &rt->duration)) {
>> +av_log(s, AV_LOG_WARNING, "Unexpected reply on
>> getStreamLength()\n");
>> +}
>> }
>>
>> fail:
>> @@ -2450,6 +2483,70 @@ static int rtmp_close(URLContext *h)
>> }
>>
>> /**
>> + * Insert a fake onMetadata packet into the FLV stream to notify the FLV
>> + * demuxer about the duration of the stream.
>> + *
>> + * This should only be done if there was no real onMetadata packet
>> sent by the
>> + * server at the start of the stream and if we were able to rezrieve
>> a valid
>
> Typo, but can be fixed before pushing
>
>> + * duration via a getStreamLength call.
>> + *
>> + * @return 0 for successful operation, negative value in case of error
>> + */
>> +static int inject_fake_duration_metadata(RTMPContext *rt)
>> +{
>> +// We need to insert the metdata packet directly after the FLV
>> +// header, i.e. we need to move all other already read data by the
>> +// size of our fake metadata packet.
>> +
>> +uint8_t* p;
>> +// Keep old flv_data pointer
>> +uint8_t* old_flv_data = rt->flv_data;
>> +// Allocate a new flv_data pointer with enough space for the
>> additional package
>> +if (!(rt->flv_data = av_malloc(rt->flv_size + 55))) {
>> +rt->flv_data = old_flv_data;
>> +return AVERROR(ENOMEM);
>> +}
>> +
>> +// Copy FLV header
>> +memcpy(rt->flv_data, old_flv_data, 13);
>> +// Copy remaining packets
>> +
Re: [libav-devel] [PATCH 3/3] rtmpproto: If stream is recorded or has a duration, we can seek
On 15/10/14 12:26, Martin Storsjö wrote:
> On Tue, 14 Oct 2014, Uwe L. Korn wrote:
>
>> ---
>> libavformat/rtmpproto.c | 6 +-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
>> index 6e90a9f..5a1e331 100644
>> --- a/libavformat/rtmpproto.c
>> +++ b/libavformat/rtmpproto.c
>> @@ -2784,6 +2784,8 @@ reconnect:
>> goto reconnect;
>> }
>>
>> +s->is_streamed = 1;
>> +
>> if (rt->is_input) {
>> int err;
>> // generate FLV header for demuxer
>> @@ -2820,6 +2822,9 @@ reconnect:
>> if ((ret = inject_fake_duration_metadata(rt)) < 0)
>> goto fail;
>> }
>> +
>> +// If the stream is recorded or we know the duration, we can
>> seek.
>> +s->is_streamed = !(!rt->live || rt->duration > 0);
>> } else {
>> rt->flv_size = 0;
>> rt->flv_data = NULL;
>> @@ -2828,7 +2833,6 @@ reconnect:
>> }
>>
>> s->max_packet_size = rt->stream->max_packet_size;
>> -s->is_streamed = 1;
>> return 0;
>>
>> fail:
>> --
>> 2.1.2
>
> No, I don't agree with this. The is_streamed flag is about whether you
> can seek in byte units (in particular, it unlocks the byte level seek
> functions within AVIOContext), and this would allow the flv demuxer to
> try weird things like "seek to the end of the file, in order to read the
> final timestamp, for figuring out the duration", which you can't. (Yes,
> normally that wouldn't happen, but e.g. if rt->live == 0 and
> rt->duration == 0, this would cause the flv demuxer to try to do such a
> seek.)
I was not fully aware that this would also affect the demuxer behaviour
like this. As I fixed the duration detection for samples with the
previous commit I was just not seeing this behaviour.
>
> Perhaps a separate signal is needed for whether the time based seek is
> usable? As a first step you can always check whether
> AVIOContext->read_seek is non-null, but something else to disambiguate
> between live and seekable rtmp might be useful.
Definitely a flag would be useful, I agree that the current patch is not
the right way, so consider this last one as "dropped".
>
> // Martin
> ___
> libav-devel mailing list
> libav-devel@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel
signature.asc
Description: OpenPGP digital signature
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
