Re: [libav-devel] [PATCH] dca: Support for XLL (lossless extension)
Diego Biurrun di...@biurrun.de writes:
I see no more issues, I just want to run this through Oracle, once the
TDSC decoder is through.
Thanks for taking care of this!
+if (chset-downmix_ncoeffs DCA_XLL_DMIX_NCOEFFS_MAX) {
+av_log(s-avctx, AV_LOG_WARNING,
+ XLL: Skipping %d downmix coefficients, exceeding
implementation limit %d\n,
+ chset-downmix_ncoeffs, DCA_XLL_DMIX_NCOEFFS_MAX);
+skip_bits_long(s-gb, 9 * chset-downmix_ncoeffs);
+chset-downmix_ncoeffs = 0;
I still think it would make sense with a return AVERROR_PATCHWELCOME
here. The alternatives are to either
1. Make the code reading chset-downmix_coeffs (that's close to the end
of ff_dca_xll_decode_audio) check for chset-downmix_ncoeffs == 0,
and do something not too insane, or error out at that time.
2. Allocate chset-downmix_coeffs dynamically.
Otherwise, we risk reading garbage, as well as reading beyond the end of
the buffer.
Regards,
/Niels
--
Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26.
Internet email is subject to wholesale government surveillance.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] dca: Support for XLL (lossless extension)
On Thu, Mar 12, 2015 at 07:41:47PM +0100, Luca Barbato wrote:
On 12/03/15 19:35, Diego Biurrun wrote:
On Thu, Mar 12, 2015 at 04:54:01PM +0100, Niels Möller wrote:
Diego Biurrun di...@biurrun.de writes:
I see no more issues, I just want to run this through Oracle, once the
TDSC decoder is through.
Thanks for taking care of this!
No issues so far, but let's wait until tomorrow for the whole run.
+if (chset-downmix_ncoeffs DCA_XLL_DMIX_NCOEFFS_MAX) {
+av_log(s-avctx, AV_LOG_WARNING,
+ XLL: Skipping %d downmix coefficients,
exceeding implementation limit %d\n,
+ chset-downmix_ncoeffs,
DCA_XLL_DMIX_NCOEFFS_MAX);
+skip_bits_long(s-gb, 9 * chset-downmix_ncoeffs);
+chset-downmix_ncoeffs = 0;
I still think it would make sense with a return AVERROR_PATCHWELCOME
here. The alternatives are to either
1. Make the code reading chset-downmix_coeffs (that's close to the end
of ff_dca_xll_decode_audio) check for chset-downmix_ncoeffs == 0,
and do something not too insane, or error out at that time.
2. Allocate chset-downmix_coeffs dynamically.
Otherwise, we risk reading garbage, as well as reading beyond the end of
the buffer.
I'll go for the return AVERROR, as in the attached patch, OK?
I'd ask for samples.
Changed locally.
Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] dca: Support for XLL (lossless extension)
On 12/03/15 19:47, Diego Biurrun wrote:
On Thu, Mar 12, 2015 at 07:41:47PM +0100, Luca Barbato wrote:
On 12/03/15 19:35, Diego Biurrun wrote:
On Thu, Mar 12, 2015 at 04:54:01PM +0100, Niels Möller wrote:
Diego Biurrun di...@biurrun.de writes:
I see no more issues, I just want to run this through Oracle, once the
TDSC decoder is through.
Thanks for taking care of this!
No issues so far, but let's wait until tomorrow for the whole run.
+if (chset-downmix_ncoeffs DCA_XLL_DMIX_NCOEFFS_MAX) {
+av_log(s-avctx, AV_LOG_WARNING,
+ XLL: Skipping %d downmix coefficients, exceeding
implementation limit %d\n,
+ chset-downmix_ncoeffs, DCA_XLL_DMIX_NCOEFFS_MAX);
+skip_bits_long(s-gb, 9 * chset-downmix_ncoeffs);
+chset-downmix_ncoeffs = 0;
I still think it would make sense with a return AVERROR_PATCHWELCOME
here. The alternatives are to either
1. Make the code reading chset-downmix_coeffs (that's close to the end
of ff_dca_xll_decode_audio) check for chset-downmix_ncoeffs == 0,
and do something not too insane, or error out at that time.
2. Allocate chset-downmix_coeffs dynamically.
Otherwise, we risk reading garbage, as well as reading beyond the end of
the buffer.
I'll go for the return AVERROR, as in the attached patch, OK?
I'd ask for samples.
Changed locally.
Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Fine for me then =)
lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] dca: Support for XLL (lossless extension)
On Thu, Mar 12, 2015 at 04:54:01PM +0100, Niels Möller wrote:
Diego Biurrun di...@biurrun.de writes:
I see no more issues, I just want to run this through Oracle, once the
TDSC decoder is through.
Thanks for taking care of this!
No issues so far, but let's wait until tomorrow for the whole run.
+if (chset-downmix_ncoeffs DCA_XLL_DMIX_NCOEFFS_MAX) {
+av_log(s-avctx, AV_LOG_WARNING,
+ XLL: Skipping %d downmix coefficients,
exceeding implementation limit %d\n,
+ chset-downmix_ncoeffs,
DCA_XLL_DMIX_NCOEFFS_MAX);
+skip_bits_long(s-gb, 9 * chset-downmix_ncoeffs);
+chset-downmix_ncoeffs = 0;
I still think it would make sense with a return AVERROR_PATCHWELCOME
here. The alternatives are to either
1. Make the code reading chset-downmix_coeffs (that's close to the end
of ff_dca_xll_decode_audio) check for chset-downmix_ncoeffs == 0,
and do something not too insane, or error out at that time.
2. Allocate chset-downmix_coeffs dynamically.
Otherwise, we risk reading garbage, as well as reading beyond the end of
the buffer.
I'll go for the return AVERROR, as in the attached patch, OK?
Diego
From 9df2aee21df40cb6376a325da8ba8a731884fafb Mon Sep 17 00:00:00 2001
From: Diego Biurrun di...@biurrun.de
Date: Thu, 12 Mar 2015 19:32:58 +0100
Subject: [PATCH] dca: Return error on too many downmix coefficients
---
libavcodec/dca_xll.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/libavcodec/dca_xll.c b/libavcodec/dca_xll.c
index 91934e3..86d8716 100644
--- a/libavcodec/dca_xll.c
+++ b/libavcodec/dca_xll.c
@@ -159,11 +159,12 @@ int ff_dca_xll_decode_header(DCAContext *s)
chset-downmix_ncoeffs = (chset-channels + 1) * s-xll_channels;
if (chset-downmix_ncoeffs DCA_XLL_DMIX_NCOEFFS_MAX) {
-av_log(s-avctx, AV_LOG_WARNING,
- XLL: Skipping %d downmix coefficients, exceeding implementation limit %d\n,
- chset-downmix_ncoeffs, DCA_XLL_DMIX_NCOEFFS_MAX);
skip_bits_long(s-gb, 9 * chset-downmix_ncoeffs);
chset-downmix_ncoeffs = 0;
+avpriv_report_missing_feature(s-avctx,
+ XLL: More than %d downmix coefficients,
+ DCA_XLL_DMIX_NCOEFFS_MAX);
+return AVERROR_PATCHWELCOME;
} else if (chset-primary_ch_set) {
for (i = 0; i chset-downmix_ncoeffs; i++)
if ((chset-downmix_coeffs[i] = dca_get_dmix_coeff(s)) == -1)
--
2.1.0
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] dca: Support for XLL (lossless extension)
Diego Biurrun di...@biurrun.de writes:
I'll go for the return AVERROR, as in the attached patch, OK?
Diego
From 9df2aee21df40cb6376a325da8ba8a731884fafb Mon Sep 17 00:00:00 2001
From: Diego Biurrun di...@biurrun.de
Date: Thu, 12 Mar 2015 19:32:58 +0100
Subject: [PATCH] dca: Return error on too many downmix coefficients
---
libavcodec/dca_xll.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/libavcodec/dca_xll.c b/libavcodec/dca_xll.c
index 91934e3..86d8716 100644
--- a/libavcodec/dca_xll.c
+++ b/libavcodec/dca_xll.c
@@ -159,11 +159,12 @@ int ff_dca_xll_decode_header(DCAContext *s)
chset-downmix_ncoeffs = (chset-channels + 1) *
s-xll_channels;
if (chset-downmix_ncoeffs DCA_XLL_DMIX_NCOEFFS_MAX) {
-av_log(s-avctx, AV_LOG_WARNING,
- XLL: Skipping %d downmix coefficients, exceeding
implementation limit %d\n,
- chset-downmix_ncoeffs, DCA_XLL_DMIX_NCOEFFS_MAX);
skip_bits_long(s-gb, 9 * chset-downmix_ncoeffs);
chset-downmix_ncoeffs = 0;
+avpriv_report_missing_feature(s-avctx,
+ XLL: More than %d downmix
coefficients,
+ DCA_XLL_DMIX_NCOEFFS_MAX);
+return AVERROR_PATCHWELCOME;
Looks OK to me. I guess you could drop the two lines before
avpriv_report_missing_feature too, if we're not going to try to go
on without the coeffs.
Regards,
/Niels
--
Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26.
Internet email is subject to wholesale government surveillance.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] dca: Support for XLL (lossless extension)
On 12/03/15 19:35, Diego Biurrun wrote:
On Thu, Mar 12, 2015 at 04:54:01PM +0100, Niels Möller wrote:
Diego Biurrun di...@biurrun.de writes:
I see no more issues, I just want to run this through Oracle, once the
TDSC decoder is through.
Thanks for taking care of this!
No issues so far, but let's wait until tomorrow for the whole run.
+if (chset-downmix_ncoeffs DCA_XLL_DMIX_NCOEFFS_MAX) {
+av_log(s-avctx, AV_LOG_WARNING,
+ XLL: Skipping %d downmix coefficients, exceeding
implementation limit %d\n,
+ chset-downmix_ncoeffs, DCA_XLL_DMIX_NCOEFFS_MAX);
+skip_bits_long(s-gb, 9 * chset-downmix_ncoeffs);
+chset-downmix_ncoeffs = 0;
I still think it would make sense with a return AVERROR_PATCHWELCOME
here. The alternatives are to either
1. Make the code reading chset-downmix_coeffs (that's close to the end
of ff_dca_xll_decode_audio) check for chset-downmix_ncoeffs == 0,
and do something not too insane, or error out at that time.
2. Allocate chset-downmix_coeffs dynamically.
Otherwise, we risk reading garbage, as well as reading beyond the end of
the buffer.
I'll go for the return AVERROR, as in the attached patch, OK?
Diego
I'd ask for samples.
lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [PATCH] dca: Support for XLL (lossless extension)
On Thu, Mar 12, 2015 at 07:41:42PM +0100, Niels Möller wrote:
Diego Biurrun di...@biurrun.de writes:
I'll go for the return AVERROR, as in the attached patch, OK?
Diego
From 9df2aee21df40cb6376a325da8ba8a731884fafb Mon Sep 17 00:00:00 2001
From: Diego Biurrun di...@biurrun.de
Date: Thu, 12 Mar 2015 19:32:58 +0100
Subject: [PATCH] dca: Return error on too many downmix coefficients
---
libavcodec/dca_xll.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/libavcodec/dca_xll.c b/libavcodec/dca_xll.c
index 91934e3..86d8716 100644
--- a/libavcodec/dca_xll.c
+++ b/libavcodec/dca_xll.c
@@ -159,11 +159,12 @@ int ff_dca_xll_decode_header(DCAContext *s)
chset-downmix_ncoeffs = (chset-channels + 1) *
s-xll_channels;
if (chset-downmix_ncoeffs DCA_XLL_DMIX_NCOEFFS_MAX) {
-av_log(s-avctx, AV_LOG_WARNING,
- XLL: Skipping %d downmix coefficients,
exceeding implementation limit %d\n,
- chset-downmix_ncoeffs,
DCA_XLL_DMIX_NCOEFFS_MAX);
skip_bits_long(s-gb, 9 * chset-downmix_ncoeffs);
chset-downmix_ncoeffs = 0;
+avpriv_report_missing_feature(s-avctx,
+ XLL: More than %d
downmix coefficients,
+
DCA_XLL_DMIX_NCOEFFS_MAX);
+return AVERROR_PATCHWELCOME;
Looks OK to me. I guess you could drop the two lines before
avpriv_report_missing_feature too, if we're not going to try to go
on without the coeffs.
Changed locally.
Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
