subject:"\[FFmpeg\-devel\] \[PATCH 2\/6\] avcodec\/vorbisenc\: Apply dynamic frame lengths"

Re: [FFmpeg-devel] [PATCH 2/6] avcodec/vorbisenc: Apply dynamic frame lengths

2017-08-23 Thread Tyler Jones

On Wed, Aug 23, 2017 at 10:31:58AM +0200, Tomas Härdin wrote:
> On 2017-08-22 03:23, Tyler Jones wrote:
> > +static int create_residues(vorbis_enc_context *venc)
> > +{
> > +int res, ret;
> > +vorbis_enc_residue *rc;
> > +
> > +venc->nresidues = 2;
> > +venc->residues  = av_malloc(sizeof(vorbis_enc_residue) * 
> > venc->nresidues);
> 
> av_malloc_array()? Applies to most av_malloc() in there

I can change it, but I don't feel that it helps readability in this specific
case above. As for the others that happen to show up in the diffs, I did not
want to make any unnecessary and unrelated functional changes. However, I'll
gladly to switch these cases to `av_malloc_array()` in a separate commit if
desired.

> > -// single mapping
> > -mc = >mappings[0];
> > -mc->submaps = 1;
> > -mc->mux = av_malloc(sizeof(int) * venc->channels);
> > -if (!mc->mux)
> > -return AVERROR(ENOMEM);
> > -for (i = 0; i < venc->channels; i++)
> > -mc->mux[i] = 0;
> > -mc->floor   = av_malloc(sizeof(int) * mc->submaps);
> > -mc->residue = av_malloc(sizeof(int) * mc->submaps);
> > -if (!mc->floor || !mc->residue)
> > -return AVERROR(ENOMEM);
> > -for (i = 0; i < mc->submaps; i++) {
> > -mc->floor[i]   = 0;
> > -mc->residue[i] = 0;
> > -}
> > -mc->coupling_steps = venc->channels == 2 ? 1 : 0;
> > -mc->magnitude  = av_malloc(sizeof(int) * mc->coupling_steps);
> > -mc->angle  = av_malloc(sizeof(int) * mc->coupling_steps);
> > -if (!mc->magnitude || !mc->angle)
> > -return AVERROR(ENOMEM);
> > -if (mc->coupling_steps) {
> > -mc->magnitude[0] = 0;
> > -mc->angle[0] = 1;
> > +for (map = 0; map < venc->nmappings; map++) {
> > +mc = >mappings[map];
> > +mc->submaps = 1;
> > +mc->mux = av_malloc(sizeof(int) * venc->channels);
> > +if (!mc->mux)
> > +return AVERROR(ENOMEM);
> > +for (i = 0; i < venc->channels; i++)
> > +mc->mux[i] = 0;
> > +mc->floor   = av_malloc(sizeof(int) * mc->submaps);
> > +mc->residue = av_malloc(sizeof(int) * mc->submaps);
> > +if (!mc->floor || !mc->residue)
> > +return AVERROR(ENOMEM);
> > +for (i = 0; i < mc->submaps; i++) {
> > +mc->floor[i]   = map;
> > +mc->residue[i] = map;
> > +}
> > +mc->coupling_steps = venc->channels == 2 ? 1 : 0;
> > +mc->magnitude  = av_malloc(sizeof(int) * mc->coupling_steps);
> > +mc->angle  = av_malloc(sizeof(int) * mc->coupling_steps);
> > +if (!mc->magnitude || !mc->angle)
> > +return AVERROR(ENOMEM);
> > +if (mc->coupling_steps) {
> > +mc->magnitude[0] = 0;
> > +mc->angle[0] = 1;
> > +}
> >   }
> 
> Maybe nitpicking, but it would be clearer what the changes are if you put
> the indentation change in a separate commit

No, you're right, and it's a good suggestion. I'll move the indentation to a
separate commit when enough other changes have been provided to warrant a new 
version.

> > -move_audio(venc, avctx->frame_size);
> > +if (venc->transient < 0) {
> > +move_audio(venc, avctx->frame_size);
> > -for (ch = 0; ch < venc->channels; ch++) {
> > -float *scratch = venc->scratch + 2 * ch * frame_size + frame_size;
> > +for (ch = 0; ch < venc->channels; ch++) {
> > +float *scratch = venc->scratch + 2 * ch * long_win + long_win;
> > -if (!ff_psy_vorbis_block_frame(>vpctx, scratch, ch,
> > -   frame_size, block_size))
> > -curr_win = 0;
> > +if (!ff_psy_vorbis_block_frame(>vpctx, scratch, ch,
> > +   long_win, short_win))
> > +next_win = 0;
> > +}
> >   }
> 
> Same here
> 
> /Tomas
> 

I felt that separating this small amount of lines would just clutter the git
log history, but I'll move these along with the mapping indentations.

Thanks for taking a look,

Tyler Jones


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/6] avcodec/vorbisenc: Apply dynamic frame lengths

2017-08-23 Thread Tomas Härdin


On 2017-08-22 03:23, Tyler Jones wrote:

+static int create_residues(vorbis_enc_context *venc)
+{
+int res, ret;
+vorbis_enc_residue *rc;
+
+venc->nresidues = 2;
+venc->residues  = av_malloc(sizeof(vorbis_enc_residue) * venc->nresidues);


av_malloc_array()? Applies to most av_malloc() in there



-// single mapping
-mc = >mappings[0];
-mc->submaps = 1;
-mc->mux = av_malloc(sizeof(int) * venc->channels);
-if (!mc->mux)
-return AVERROR(ENOMEM);
-for (i = 0; i < venc->channels; i++)
-mc->mux[i] = 0;
-mc->floor   = av_malloc(sizeof(int) * mc->submaps);
-mc->residue = av_malloc(sizeof(int) * mc->submaps);
-if (!mc->floor || !mc->residue)
-return AVERROR(ENOMEM);
-for (i = 0; i < mc->submaps; i++) {
-mc->floor[i]   = 0;
-mc->residue[i] = 0;
-}
-mc->coupling_steps = venc->channels == 2 ? 1 : 0;
-mc->magnitude  = av_malloc(sizeof(int) * mc->coupling_steps);
-mc->angle  = av_malloc(sizeof(int) * mc->coupling_steps);
-if (!mc->magnitude || !mc->angle)
-return AVERROR(ENOMEM);
-if (mc->coupling_steps) {
-mc->magnitude[0] = 0;
-mc->angle[0] = 1;
+for (map = 0; map < venc->nmappings; map++) {
+mc = >mappings[map];
+mc->submaps = 1;
+mc->mux = av_malloc(sizeof(int) * venc->channels);
+if (!mc->mux)
+return AVERROR(ENOMEM);
+for (i = 0; i < venc->channels; i++)
+mc->mux[i] = 0;
+mc->floor   = av_malloc(sizeof(int) * mc->submaps);
+mc->residue = av_malloc(sizeof(int) * mc->submaps);
+if (!mc->floor || !mc->residue)
+return AVERROR(ENOMEM);
+for (i = 0; i < mc->submaps; i++) {
+mc->floor[i]   = map;
+mc->residue[i] = map;
+}
+mc->coupling_steps = venc->channels == 2 ? 1 : 0;
+mc->magnitude  = av_malloc(sizeof(int) * mc->coupling_steps);
+mc->angle  = av_malloc(sizeof(int) * mc->coupling_steps);
+if (!mc->magnitude || !mc->angle)
+return AVERROR(ENOMEM);
+if (mc->coupling_steps) {
+mc->magnitude[0] = 0;
+mc->angle[0] = 1;
+}
  }


Maybe nitpicking, but it would be clearer what the changes are if you 
put the indentation change in a separate commit




-move_audio(venc, avctx->frame_size);
+if (venc->transient < 0) {
+move_audio(venc, avctx->frame_size);
  
-for (ch = 0; ch < venc->channels; ch++) {

-float *scratch = venc->scratch + 2 * ch * frame_size + frame_size;
+for (ch = 0; ch < venc->channels; ch++) {
+float *scratch = venc->scratch + 2 * ch * long_win + long_win;
  
-if (!ff_psy_vorbis_block_frame(>vpctx, scratch, ch,

-   frame_size, block_size))
-curr_win = 0;
+if (!ff_psy_vorbis_block_frame(>vpctx, scratch, ch,
+   long_win, short_win))
+next_win = 0;
+}
  }


Same here

/Tomas

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 2/6] avcodec/vorbisenc: Apply dynamic frame lengths

2017-08-21 Thread Tyler Jones

Additional codebooks are added for shorter 128-sample frames. Changes in
codeword generation are made to handle valid values of 0 that prepend some
codebooks, otherwise books are classified incorrectly and cause unreadable
streams.

A second residue, floor, and mapping is created for short window lengths
so that values are partitioned correctly for transient frames.

Signed-off-by: Tyler Jones 
---
V4: No changes
V3: Switch 'bits[p] == 0' to '!bits[p]' in vlc gen
V2: Fix double arithmetic in window scale

 libavcodec/vorbis.c  |  10 +-
 libavcodec/vorbis_enc_data.h | 289 +++--
 libavcodec/vorbisenc.c   | 424 ++-
 tests/fate/vorbis.mak|   2 +-
 4 files changed, 454 insertions(+), 271 deletions(-)

diff --git a/libavcodec/vorbis.c b/libavcodec/vorbis.c
index 399020eec5..d8c4b006e7 100644
--- a/libavcodec/vorbis.c
+++ b/libavcodec/vorbis.c
@@ -59,7 +59,7 @@ int ff_vorbis_len2vlc(uint8_t *bits, uint32_t *codes, 
unsigned num)
 unsigned i, j, p, code;
 
 for (p = 0; (bits[p] == 0) && (p < num); ++p)
-;
+codes[p] = 0;
 if (p == num)
 return 0;
 
@@ -78,9 +78,11 @@ int ff_vorbis_len2vlc(uint8_t *bits, uint32_t *codes, 
unsigned num)
 
 for (; p < num; ++p) {
 if (bits[p] > 32)
- return AVERROR_INVALIDDATA;
-if (bits[p] == 0)
- continue;
+return AVERROR_INVALIDDATA;
+if (!bits[p]) {
+codes[p] = 0;
+continue;
+}
 // find corresponding exit(node which the tree can grow further from)
 for (i = bits[p]; i > 0; --i)
 if (exit_at_level[i])
diff --git a/libavcodec/vorbis_enc_data.h b/libavcodec/vorbis_enc_data.h
index a51aaec978..eca43dfded 100644
--- a/libavcodec/vorbis_enc_data.h
+++ b/libavcodec/vorbis_enc_data.h
@@ -23,15 +23,78 @@
 
 #include 
 
-static const uint8_t codebook0[] = {
+static const uint8_t floor_128_c0[] = {
+10,  7,  8, 13,  9,  6,  7, 11, 10,  8,  8, 12, 17, 17, 17,
+17,  7,  5,  5,  9,  6,  4,  4,  8,  8,  5,  5,  8, 16, 14,
+13, 16,  7,  5,  5,  7,  6,  3,  3,  5,  8,  5,  4,  7, 14,
+12, 12, 15, 10,  7,  8,  9,  7,  5,  5,  6,  9,  6,  5,  5,
+15, 12,  9, 10,
+};
+
+static const uint8_t floor_128_c1[] = {
+ 8, 13, 17, 17,  8, 11, 17, 17, 11, 13, 17, 17, 17, 17, 17,
+17,  6, 10, 16, 17,  6, 10, 15, 17,  8, 10, 16, 17, 17, 17,
+17, 17,  9, 13, 15, 17,  8, 11, 17, 17, 10, 12, 17, 17, 17,
+17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 17,
+17, 17, 17, 17,  6, 11, 15, 17,  7, 10, 15, 17,  8, 10, 17,
+17, 17, 15, 17, 17,  4,  8, 13, 17,  4,  7, 13, 17,  6,  8,
+15, 17, 16, 15, 17, 17,  6, 11, 15, 17,  6,  9, 13, 17,  8,
+10, 17, 17, 15, 17, 17, 17, 16, 17, 17, 17, 12, 14, 15, 17,
+13, 14, 15, 17, 17, 17, 17, 17,  5, 10, 14, 17,  5,  9, 14,
+17,  7,  9, 15, 17, 15, 15, 17, 17,  3,  7, 12, 17,  3,  6,
+11, 17,  5,  7, 13, 17, 12, 12, 17, 17,  5,  9, 14, 17,  3,
+ 7, 11, 17,  5,  8, 13, 17, 13, 11, 16, 17, 12, 17, 17, 17,
+ 9, 14, 15, 17, 10, 11, 14, 17, 16, 14, 17, 17,  8, 12, 17,
+17,  8, 12, 17, 17, 10, 12, 17, 17, 17, 17, 17, 17,  5, 10,
+17, 17,  5,  9, 15, 17,  7,  9, 17, 17, 13, 13, 17, 17,  7,
+11, 17, 17,  6, 10, 15, 17,  7,  9, 15, 17, 12, 11, 17, 17,
+12, 15, 17, 17, 11, 14, 17, 17, 11, 10, 15, 17, 17, 16, 17,
+17,
+};
+
+static const uint8_t floor_128_0sub1[] = {
+ 0,  3,  3,  3,  3,  3,  3,  3,  3,
+};
+
+static const uint8_t floor_128_0sub2[] = {
+ 0,  0,  0,  0,  0,  0,  0,  0,  0,  3,  3,  3,  4,  4,  4,
+ 4,  5,  4,  5,  4,  5,  4,  6,  4,  6,
+};
+
+static const uint8_t floor_128_0sub3[] = {
+ 0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
+ 0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  3,  5,  3,  5,  3,
+ 5,  4,  5,  4,  5,  5,  5,  5,  6,  5,  6,  5,  6,  5,  6,
+ 5,  6,  5,  7,  8,  9, 11, 13, 13, 13, 13, 13, 13, 13, 13,
+13, 13, 13, 13,
+};
+
+static const uint8_t floor_128_1sub1[] = {
+ 0,  3,  3,  2,  3,  3,  4,  3,  4,
+};
+
+static const uint8_t floor_128_1sub2[] = {
+ 0,  0,  0,  0,  0,  0,  0,  0,  0,  3,  4,  3,  6,  3,  6,
+ 3,  6,  3,  7,  3,  8,  4,  9,  4,  9,
+};
+
+static const uint8_t floor_128_1sub3[] = {
+ 0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
+ 0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  1,  7,  2,  7,  3,
+ 8,  4,  9,  5,  9,  8, 10, 11, 11, 12, 14, 14, 14, 14, 14,
+14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14,
+13, 13, 13, 13,
+};
+
+static const uint8_t floor_1024_c1[] = {
 2, 10,  8, 14,  7, 12, 11, 14,  1,  5,  3,  7,  4,  9,  7, 13,
 };
 
-static const uint8_t codebook1[] = {
+static const uint8_t floor_1024_c2[] = {
 1,  4,  2,  6,  3,  7,  5,  7,
 };
 
-static const uint8_t codebook2[] = {
+static const uint8_t floor_1024_c3[] = {
  1,  5,  7, 21,  5,  8,  9, 21, 10,  9, 12, 20, 20,

Re: [FFmpeg-devel] [PATCH 2/6] avcodec/vorbisenc: Apply dynamic frame lengths

Re: [FFmpeg-devel] [PATCH 2/6] avcodec/vorbisenc: Apply dynamic frame lengths

[FFmpeg-devel] [PATCH 2/6] avcodec/vorbisenc: Apply dynamic frame lengths

3 matches

Site Navigation

Mail list logo

Footer information