[FFmpeg-devel] [PATCH] libavcodec/tests: Added test for libavcodec/avpacket.c

[Michael Niedermayer]

On Thu, Nov 03, 2016 at 04:55:40AM -0700, Thomas Turner wrote:
> Function(s) Tested: av_packet_clone().
> 
> This test checks if av_packet_clone() can successfully make a copy of an
> AVPacket. Compares all data members in AVPacket EXCEPT for "buf" because "buf" is
> initialized to NIL in the original AVPacket [to be cloned].
> 
> Signed-off-by: Thomas Turner <thomastdt at googlemail.com>
> ---
>  libavcodec/Makefile         |   3 +-
>  libavcodec/tests/avpacket.c | 205 ++++++++++++++++++++++++++++++++++++++++++++
>  tests/fate/libavcodec.mak   |   5 ++
>  3 files changed, 212 insertions(+), 1 deletion(-)
>  create mode 100644 libavcodec/tests/avpacket.c
> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index f1d5bf1..46e3af7 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -1019,7 +1019,8 @@ SKIPHEADERS-$(CONFIG_VDA)              += vda.h vda_vt_internal.h
>  SKIPHEADERS-$(CONFIG_VDPAU)            += vdpau.h vdpau_internal.h
>  SKIPHEADERS-$(CONFIG_VIDEOTOOLBOX)     += videotoolbox.h vda_vt_internal.h
>  
> -TESTPROGS = imgconvert                                                  \
> +TESTPROGS = avpacket                                                    \
> +            imgconvert                                                  \
>              jpeg2000dwt                                                 \
>              mathops                                                    \
>              options                                                     \
> diff --git a/libavcodec/tests/avpacket.c b/libavcodec/tests/avpacket.c
> new file mode 100644
> index 0000000..784a5a4
> --- /dev/null
> +++ b/libavcodec/tests/avpacket.c
> @@ -0,0 +1,205 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <inttypes.h>
> +#include <string.h>
> +#include "libavcodec/avcodec.h"
> +#include "libavutil/error.h"
> +
> +
> +
> +static const char* str = "Error @";
> +
> +static int compare_AVPacketsSideData(AVPacketSideData* sdata1,
> +                                     AVPacketSideData* sdata2)
> +{
> +    int ret = 0;
> +
> +    if(!sdata1 || !sdata2)
> +        return;
> +    av_log(NULL, AV_LOG_INFO, "Comparing sidedata data memebers...\n");
> +
> +    if(sdata1->size != sdata2->size){
> +        fprintf(stderr, "%s size\n", str);
> +        ret = 1;
> +    }
> +    if(sdata1->size > 0){
> +        if(memcmp(sdata1->data, sdata2->data, sdata1->size) != 0){
> +            fprintf(stderr, "%s data\n", str);
> +            ret = 1;
> +        }
> +    }
> +    else {
> +        av_log(NULL, AV_LOG_INFO, "size is <= 0");
> +        ret = 1;
> +    }
> +    if(sdata1->type != sdata2->type){
> +            fprintf(stderr, "%s type\n", str);
> +            ret = 1;
> +        }

indention is wrong here

also forcing a failure in one of these if() does not cause the
fate test to fail, the test still succeeds as it returns 0 and its
stdout/err output is not checked

make fate-avpacket
TEST    avpacket

Also i think the whole test is much more verbose than needed.

I think spending time on testing more functions makes more sense
than analysing the error case of one.
Code to analyze errors can be written if we ever hit an error.

Its different with tests outputing checksums, there details are
usefull so the developer can determine if a change is an error or a
intended consequence of some change to the codebase.
Here simply saying "error" if theres a difference is fine.
Code to print the details can be written if it actually gets hit.


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

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161103/8ae4ab59/attachment.sig>