Hi Tom,
On Fri, Jul 17, 2015 at 1:11 PM, Tom Butterworth <[email protected]> wrote:
> This change will reject frames with a texture type which doesn't match the
> stream description.
Is this change required by the format specification or just as a
precaution for malformed streams?
> ---
> libavcodec/hapdec.c | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/libavcodec/hapdec.c b/libavcodec/hapdec.c
> index ca1c158..324a43a 100644
> --- a/libavcodec/hapdec.c
> +++ b/libavcodec/hapdec.c
> @@ -78,20 +78,19 @@ static int setup_texture(AVCodecContext *avctx, size_t
> length)
> const char *compressorstr;
> int ret;
>
> + if ((avctx->codec_tag == MKTAG('H','a','p','1') && (ctx->section_type &
> 0x0F) != HAP_FMT_RGBDXT1)
> + || (avctx->codec_tag == MKTAG('H','a','p','5') && (ctx->section_type
> & 0x0F) != HAP_FMT_RGBADXT5)
> + || (avctx->codec_tag == MKTAG('H','a','p','Y') && (ctx->section_type
> & 0x0F) != HAP_FMT_YCOCGDXT5))
> + return AVERROR_INVALIDDATA;
can you please add an error message so that the user knows what went wrong?
style nit: please keep || at the end of the line, and the () around
conditions can be omitted
> switch (ctx->section_type & 0x0F) {
> case HAP_FMT_RGBDXT1:
> - ctx->tex_rat = 8;
> - ctx->tex_fun = ctx->dxtc.dxt1_block;
> texture_name = "DXT1";
> break;
> case HAP_FMT_RGBADXT5:
> - ctx->tex_rat = 16;
> - ctx->tex_fun = ctx->dxtc.dxt5_block;
> texture_name = "DXT5";
> break;
> case HAP_FMT_YCOCGDXT5:
> - ctx->tex_rat = 16;
> - ctx->tex_fun = ctx->dxtc.dxt5ys_block;
> texture_name = "DXT5-YCoCg-scaled";
> break;
> default:
> @@ -211,6 +210,22 @@ static av_cold int hap_init(AVCodecContext *avctx)
>
> ff_texturedsp_init(&ctx->dxtc);
>
> + switch (avctx->codec_tag) {
> + case MKTAG('H','a','p','1'):
> + ctx->tex_rat = 8;
> + ctx->tex_fun = ctx->dxtc.dxt1_block;
> + break;
> + case MKTAG('H','a','p','5'):
> + ctx->tex_rat = 16;
> + ctx->tex_fun = ctx->dxtc.dxt5_block;
> + break;
> + case MKTAG('H','a','p','Y'):
> + ctx->tex_rat = 16;
> + ctx->tex_fun = ctx->dxtc.dxt5ys_block;
> + break;
> + default:
> + return AVERROR_DECODER_NOT_FOUND;
Is this change necessary? There are now two switches that carry out
the same purpose and the condition above forces cases from both
switches to perform the same operation.
--
Vittorio
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
