On Tue, Jul 21, 2015 at 1:21 AM, Tom Butterworth <[email protected]> wrote:
> ---
> libavcodec/Makefile | 4 +-
> libavcodec/hap.c | 55 ++++++++++
> libavcodec/hap.h | 37 ++++++-
> libavcodec/hapdec.c | 272
> +++++++++++++++++++++++++++++++++++++----------
> libavcodec/hapenc.c | 187 ++++++++++++++++++++++++++------
> tests/fate/video.mak | 3 +
> tests/ref/fate/hap-chunk | 2 +
> 7 files changed, 464 insertions(+), 96 deletions(-)
> create mode 100644 libavcodec/hap.c
> create mode 100644 tests/ref/fate/hap-chunk
Patches 1, 2 and 3 are good to me (minus a few style nits I can take care of).
I'd still prefer this one split in encoder/decoder but I won't insist
if you prefer otherwise.
However when trying this patch I noticed it doesn't decode the last 4
lines: see the attached picture for what I mean.
I think it's due to the decoding changes in
977105407cae55876041dddbf4ce0934cdd4cd6c.
> diff --git a/libavcodec/hapenc.c b/libavcodec/hapenc.c
> index d828a09..bf28060 100644
> --- a/libavcodec/hapenc.c
> +++ b/libavcodec/hapenc.c
> @@ -42,8 +43,12 @@
> #include "internal.h"
> #include "texturedsp.h"
>
> -/* A fixed header size allows to skip a memcpy */
> -#define HEADER_SIZE 8
> +#define HAP_MAX_CHUNKS 64
> +
> +enum HapHeaderLength {
> + HAP_HDR_4_BYTE = 4,
> + HAP_HDR_8_BYTE = 8,
> +};
imho no need to indicate that it's a BYTE, I
> +static int hap_decode_instructions_length(HapContext *ctx)
> +{
> + /* = Second-Stage Compressor Table + Chunk Size Table + headers for both
> sections
> + * = chunk_count + (4 * chunk_count) + 4 + 4 */
> + return (5 * ctx->chunk_count) + 8;
> +}
it took me a little to figure out this comment, could you maybe add
the "Length" variable before '=' (or just drop '=')?
Cheers
--
Vittorio
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
