[FFmpeg-devel] [PATCH 1/2] opus/matroska: Adding support for DiscardPadding in muxer

[Michael Niedermayer]

On Tue, Aug 20, 2013 at 03:13:36PM -0700, Vignesh Venkatasubramanian wrote:
> Support for end trimming Opus in Matroska is implemented by using
> the DiscardPadding container element in the Block data. The last
> chunk is stored as a Block instead of SimpleBlock and the
> trimming information is stored and used to discard samples that
> were padded by the Opus codec. This patch adds support for muxing
> DiscardPadding element into the container with appropriate value.
> Matroska spec for the DiscardPadding element can be found here:
> http://matroska.org/technical/specs/index.html#DiscardPadding
> 
> Signed-off-by: Vignesh Venkatasubramanian <vigneshv at google.com>
> ---
>  libavcodec/avpacket.c     |  2 --
>  libavcodec/libopusenc.c   | 12 ++++++++++++
>  libavformat/matroska.h    |  1 +
>  libavformat/matroskaenc.c | 24 ++++++++++++++++++++++--
>  4 files changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index 7196c31..ac761f1 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -225,8 +225,6 @@ int av_copy_packet_side_data(AVPacket *pkt, AVPacket *src)
>          int i;
>          DUP_DATA(pkt->side_data, src->side_data,
>                  src->side_data_elems * sizeof(*src->side_data), 0, ALLOC_MALLOC);
> -        memset(pkt->side_data, 0,
> -                src->side_data_elems * sizeof(*src->side_data));
>          for (i = 0; i < src->side_data_elems; i++) {
>              DUP_DATA(pkt->side_data[i].data, src->side_data[i].data,
>                      src->side_data[i].size, 1, ALLOC_MALLOC);

i see why you need this but simply removing the memset could leave
the packets in a invalid state on error conditions.
From a quick look this looks like it could then lead to double frees

Also this belongs in a seperate patch



> diff --git a/libavcodec/libopusenc.c b/libavcodec/libopusenc.c
> index 59caac7..775b2ef 100644
> --- a/libavcodec/libopusenc.c
> +++ b/libavcodec/libopusenc.c
> @@ -354,6 +354,18 @@ static int libopus_encode(AVCodecContext *avctx, AVPacket *avpkt,
>      ff_af_queue_remove(&opus->afq, opus->opts.packet_size,
>                         &avpkt->pts, &avpkt->duration);
>  

> +    if (opus->opts.packet_size - avpkt->duration > 0) {

could the subtraction overflow ?


> +        uint8_t* side_data = av_packet_new_side_data(avpkt,
> +                                                     AV_PKT_DATA_SKIP_SAMPLES,
> +                                                     10);
> +        if(side_data == NULL) {
> +            av_free_packet(avpkt);
> +            av_free(avpkt);
> +            return AVERROR(ENOMEM);
> +        }
> +        AV_WL32(side_data + 4, opus->opts.packet_size - avpkt->duration);
> +    }
> +
>      *got_packet_ptr = 1;
>  
>      return 0;

this too belongs in a seperate patch

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130830/30606576/attachment.asc>