diff mbox series

[FFmpeg-devel,2/2] avcodec/g723_1enc: Avoid skip_put_bits()

Message ID 20200731103946.2843-2-andreas.rheinhardt@gmail.com
State Accepted
Commit 15ef16dce8b69200a972a0ea6aa9026ab79e09ad
Headers show
Series [FFmpeg-devel,1/2] avcodec/put_bits: Make skip_put_bits() less dangerous
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt July 31, 2020, 10:39 a.m. UTC
If a bit is reserved, it matters very much what value it has, because
otherwise a decoder conforming to a future version of the standard might
interpret the output file in an unintended manner. This implies that
one must not use skip_put_bits() for it (which does not give any
guarantees wrt what ends up in the output (in case of a little-endian
bitstream writer (as here) it writes a 0 bit)); given that the reference
encoder as well as the earlier code write a zero bit at this place, the
new code does, too.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
The earlier code here was unaffected by any of the bugs of
skip_put_bits, because this code uses a little-endian writer and
the number of bits written so far was always even, so that skipping
a single bit could be done in the buffer itself.

 libavcodec/g723_1enc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michael Niedermayer July 31, 2020, 7:38 p.m. UTC | #1
On Fri, Jul 31, 2020 at 12:39:46PM +0200, Andreas Rheinhardt wrote:
> If a bit is reserved, it matters very much what value it has, because
> otherwise a decoder conforming to a future version of the standard might
> interpret the output file in an unintended manner. This implies that
> one must not use skip_put_bits() for it (which does not give any
> guarantees wrt what ends up in the output (in case of a little-endian
> bitstream writer (as here) it writes a 0 bit)); given that the reference
> encoder as well as the earlier code write a zero bit at this place, the
> new code does, too.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> The earlier code here was unaffected by any of the bugs of
> skip_put_bits, because this code uses a little-endian writer and
> the number of bits written so far was always even, so that skipping
> a single bit could be done in the buffer itself.
> 
>  libavcodec/g723_1enc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

LGTM

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/g723_1enc.c b/libavcodec/g723_1enc.c
index 592840566e..b2ba3c2230 100644
--- a/libavcodec/g723_1enc.c
+++ b/libavcodec/g723_1enc.c
@@ -1030,7 +1030,7 @@  static int pack_bitstream(G723_1_ChannelContext *p, AVPacket *avpkt)
     put_bits(&pb, 1, p->subframe[3].grid_index);
 
     if (p->cur_rate == RATE_6300) {
-        skip_put_bits(&pb, 1); /* reserved bit */
+        put_bits(&pb, 1, 0); /* reserved bit */
 
         /* Write 13 bit combined position index */
         temp = (p->subframe[0].pulse_pos >> 16) * 810 +