| 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 | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
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 --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 +
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(-)