| Message ID | 20200101005837.11356-3-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Matroska muxer patches | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
Am Mi., 1. Jan. 2020 um 01:59 Uhr schrieb Andreas Rheinhardt <andreas.rheinhardt@gmail.com>: > > The Matroska Projection master element has such a small maximum length > that it can always be written with a length field of length one. > So it is unnecessary to first write the element into a dynamic buffer to > get the accurate length in order not to waste bytes on the length field. "Improve" or "Simplify"?
On Wed, Jan 1, 2020 at 4:53 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > Am Mi., 1. Jan. 2020 um 01:59 Uhr schrieb Andreas Rheinhardt > <andreas.rheinhardt@gmail.com>: > > > > The Matroska Projection master element has such a small maximum length > > that it can always be written with a length field of length one. > > So it is unnecessary to first write the element into a dynamic buffer to > > get the accurate length in order not to waste bytes on the length field. > > "Improve" or "Simplify"? > > Both are applicable. - Andreas
On Wed, Jan 1, 2020 at 1:59 AM Andreas Rheinhardt < andreas.rheinhardt@gmail.com> wrote: > The Matroska Projection master element has such a small maximum length > that it can always be written with a length field of length one. > So it is unnecessary to first write the element into a dynamic buffer to > get the accurate length in order not to waste bytes on the length field. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavformat/matroskaenc.c | 48 +++++++++++++++++---------------------- > 1 file changed, 21 insertions(+), 27 deletions(-) > > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c > index 9cf840c9be..bbf9b55e78 100644 > --- a/libavformat/matroskaenc.c > +++ b/libavformat/matroskaenc.c > @@ -947,10 +947,8 @@ static int mkv_write_video_projection(AVFormatContext > *s, AVIOContext *pb, > AVStream *st) > { > AVIOContext b; > - AVIOContext *dyn_cp; > + ebml_master projection; > int side_data_size = 0; > - int ret, projection_size; > - uint8_t *projection_ptr; > uint8_t private[20]; > > const AVSphericalMapping *spherical = > @@ -960,62 +958,58 @@ static int > mkv_write_video_projection(AVFormatContext *s, AVIOContext *pb, > if (!side_data_size) > return 0; > > - ret = avio_open_dyn_buf(&dyn_cp); > - if (ret < 0) > - return ret; > + if (spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR && > + spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR_TILE && > + spherical->projection != AV_SPHERICAL_CUBEMAP) { > + av_log(s, AV_LOG_WARNING, "Unknown projection type\n"); > + return 0; > + } > + > + // Maximally 4 8-byte elements with id-length 2 + 1 byte length field > + // and the private data of the AV_SPHERICAL_EQUIRECTANGULAR_TILE case > + projection = start_ebml_master(pb, MATROSKA_ID_VIDEOPROJECTION, > + 4 * (2 + 1 + 8) + (2 + 1 + 20)); > > switch (spherical->projection) { > case AV_SPHERICAL_EQUIRECTANGULAR: > - put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE, > + put_ebml_uint(pb, MATROSKA_ID_VIDEOPROJECTIONTYPE, > MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR); > break; > case AV_SPHERICAL_EQUIRECTANGULAR_TILE: > ffio_init_context(&b, private, 20, 1, NULL, NULL, NULL, NULL); > - put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE, > + put_ebml_uint(pb, MATROSKA_ID_VIDEOPROJECTIONTYPE, > MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR); > avio_wb32(&b, 0); // version + flags > avio_wb32(&b, spherical->bound_top); > avio_wb32(&b, spherical->bound_bottom); > avio_wb32(&b, spherical->bound_left); > avio_wb32(&b, spherical->bound_right); > - put_ebml_binary(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPRIVATE, > + put_ebml_binary(pb, MATROSKA_ID_VIDEOPROJECTIONPRIVATE, > private, avio_tell(&b)); > break; > case AV_SPHERICAL_CUBEMAP: > ffio_init_context(&b, private, 12, 1, NULL, NULL, NULL, NULL); > - put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE, > + put_ebml_uint(pb, MATROSKA_ID_VIDEOPROJECTIONTYPE, > MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP); > avio_wb32(&b, 0); // version + flags > avio_wb32(&b, 0); // layout > avio_wb32(&b, spherical->padding); > - put_ebml_binary(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPRIVATE, > + put_ebml_binary(pb, MATROSKA_ID_VIDEOPROJECTIONPRIVATE, > private, avio_tell(&b)); > break; > - default: > - av_log(s, AV_LOG_WARNING, "Unknown projection type\n"); > - goto end; > } > > if (spherical->yaw) > - put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEYAW, > + put_ebml_float(pb, MATROSKA_ID_VIDEOPROJECTIONPOSEYAW, > (double) spherical->yaw / (1 << 16)); > if (spherical->pitch) > - put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEPITCH, > + put_ebml_float(pb, MATROSKA_ID_VIDEOPROJECTIONPOSEPITCH, > (double) spherical->pitch / (1 << 16)); > if (spherical->roll) > - put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEROLL, > + put_ebml_float(pb, MATROSKA_ID_VIDEOPROJECTIONPOSEROLL, > (double) spherical->roll / (1 << 16)); > > -end: > - projection_size = avio_close_dyn_buf(dyn_cp, &projection_ptr); > - if (projection_size) { > - ebml_master projection = start_ebml_master(pb, > - > MATROSKA_ID_VIDEOPROJECTION, > - projection_size); > - avio_write(pb, projection_ptr, projection_size); > - end_ebml_master(pb, projection); > - } > - av_freep(&projection_ptr); > + end_ebml_master(pb, projection); > > return 0; > } > -- > 2.20.1 > > Ping. - Andreas
On 12/31/2019 9:58 PM, Andreas Rheinhardt wrote: > The Matroska Projection master element has such a small maximum length > that it can always be written with a length field of length one. > So it is unnecessary to first write the element into a dynamic buffer to > get the accurate length in order not to waste bytes on the length field. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavformat/matroskaenc.c | 48 +++++++++++++++++---------------------- > 1 file changed, 21 insertions(+), 27 deletions(-) > > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c > index 9cf840c9be..bbf9b55e78 100644 > --- a/libavformat/matroskaenc.c > +++ b/libavformat/matroskaenc.c > @@ -947,10 +947,8 @@ static int mkv_write_video_projection(AVFormatContext *s, AVIOContext *pb, > AVStream *st) > { > AVIOContext b; > - AVIOContext *dyn_cp; > + ebml_master projection; > int side_data_size = 0; > - int ret, projection_size; > - uint8_t *projection_ptr; > uint8_t private[20]; > > const AVSphericalMapping *spherical = > @@ -960,62 +958,58 @@ static int mkv_write_video_projection(AVFormatContext *s, AVIOContext *pb, > if (!side_data_size) > return 0; > > - ret = avio_open_dyn_buf(&dyn_cp); > - if (ret < 0) > - return ret; > + if (spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR && > + spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR_TILE && > + spherical->projection != AV_SPHERICAL_CUBEMAP) { > + av_log(s, AV_LOG_WARNING, "Unknown projection type\n"); > + return 0; > + } > + > + // Maximally 4 8-byte elements with id-length 2 + 1 byte length field > + // and the private data of the AV_SPHERICAL_EQUIRECTANGULAR_TILE case > + projection = start_ebml_master(pb, MATROSKA_ID_VIDEOPROJECTION, > + 4 * (2 + 1 + 8) + (2 + 1 + 20)); > > switch (spherical->projection) { > case AV_SPHERICAL_EQUIRECTANGULAR: > - put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE, > + put_ebml_uint(pb, MATROSKA_ID_VIDEOPROJECTIONTYPE, > MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR); > break; > case AV_SPHERICAL_EQUIRECTANGULAR_TILE: > ffio_init_context(&b, private, 20, 1, NULL, NULL, NULL, NULL); > - put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE, > + put_ebml_uint(pb, MATROSKA_ID_VIDEOPROJECTIONTYPE, > MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR); > avio_wb32(&b, 0); // version + flags > avio_wb32(&b, spherical->bound_top); > avio_wb32(&b, spherical->bound_bottom); > avio_wb32(&b, spherical->bound_left); > avio_wb32(&b, spherical->bound_right); > - put_ebml_binary(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPRIVATE, > + put_ebml_binary(pb, MATROSKA_ID_VIDEOPROJECTIONPRIVATE, > private, avio_tell(&b)); > break; > case AV_SPHERICAL_CUBEMAP: > ffio_init_context(&b, private, 12, 1, NULL, NULL, NULL, NULL); > - put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE, > + put_ebml_uint(pb, MATROSKA_ID_VIDEOPROJECTIONTYPE, > MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP); > avio_wb32(&b, 0); // version + flags > avio_wb32(&b, 0); // layout > avio_wb32(&b, spherical->padding); > - put_ebml_binary(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPRIVATE, > + put_ebml_binary(pb, MATROSKA_ID_VIDEOPROJECTIONPRIVATE, > private, avio_tell(&b)); > break; > - default: > - av_log(s, AV_LOG_WARNING, "Unknown projection type\n"); > - goto end; Changed this to av_assert0(0) since the check you added above should ensure a "default" path is never taken here, and pushed. Thanks. > } > > if (spherical->yaw) > - put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEYAW, > + put_ebml_float(pb, MATROSKA_ID_VIDEOPROJECTIONPOSEYAW, > (double) spherical->yaw / (1 << 16)); > if (spherical->pitch) > - put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEPITCH, > + put_ebml_float(pb, MATROSKA_ID_VIDEOPROJECTIONPOSEPITCH, > (double) spherical->pitch / (1 << 16)); > if (spherical->roll) > - put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEROLL, > + put_ebml_float(pb, MATROSKA_ID_VIDEOPROJECTIONPOSEROLL, > (double) spherical->roll / (1 << 16)); > > -end: > - projection_size = avio_close_dyn_buf(dyn_cp, &projection_ptr); > - if (projection_size) { > - ebml_master projection = start_ebml_master(pb, > - MATROSKA_ID_VIDEOPROJECTION, > - projection_size); > - avio_write(pb, projection_ptr, projection_size); > - end_ebml_master(pb, projection); > - } > - av_freep(&projection_ptr); > + end_ebml_master(pb, projection); > > return 0; > } >
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 9cf840c9be..bbf9b55e78 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -947,10 +947,8 @@ static int mkv_write_video_projection(AVFormatContext *s, AVIOContext *pb, AVStream *st) { AVIOContext b; - AVIOContext *dyn_cp; + ebml_master projection; int side_data_size = 0; - int ret, projection_size; - uint8_t *projection_ptr; uint8_t private[20]; const AVSphericalMapping *spherical = @@ -960,62 +958,58 @@ static int mkv_write_video_projection(AVFormatContext *s, AVIOContext *pb, if (!side_data_size) return 0; - ret = avio_open_dyn_buf(&dyn_cp); - if (ret < 0) - return ret; + if (spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR && + spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR_TILE && + spherical->projection != AV_SPHERICAL_CUBEMAP) { + av_log(s, AV_LOG_WARNING, "Unknown projection type\n"); + return 0; + } + + // Maximally 4 8-byte elements with id-length 2 + 1 byte length field + // and the private data of the AV_SPHERICAL_EQUIRECTANGULAR_TILE case + projection = start_ebml_master(pb, MATROSKA_ID_VIDEOPROJECTION, + 4 * (2 + 1 + 8) + (2 + 1 + 20)); switch (spherical->projection) { case AV_SPHERICAL_EQUIRECTANGULAR: - put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE, + put_ebml_uint(pb, MATROSKA_ID_VIDEOPROJECTIONTYPE, MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR); break; case AV_SPHERICAL_EQUIRECTANGULAR_TILE: ffio_init_context(&b, private, 20, 1, NULL, NULL, NULL, NULL); - put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE, + put_ebml_uint(pb, MATROSKA_ID_VIDEOPROJECTIONTYPE, MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR); avio_wb32(&b, 0); // version + flags avio_wb32(&b, spherical->bound_top); avio_wb32(&b, spherical->bound_bottom); avio_wb32(&b, spherical->bound_left); avio_wb32(&b, spherical->bound_right); - put_ebml_binary(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPRIVATE, + put_ebml_binary(pb, MATROSKA_ID_VIDEOPROJECTIONPRIVATE, private, avio_tell(&b)); break; case AV_SPHERICAL_CUBEMAP: ffio_init_context(&b, private, 12, 1, NULL, NULL, NULL, NULL); - put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE, + put_ebml_uint(pb, MATROSKA_ID_VIDEOPROJECTIONTYPE, MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP); avio_wb32(&b, 0); // version + flags avio_wb32(&b, 0); // layout avio_wb32(&b, spherical->padding); - put_ebml_binary(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPRIVATE, + put_ebml_binary(pb, MATROSKA_ID_VIDEOPROJECTIONPRIVATE, private, avio_tell(&b)); break; - default: - av_log(s, AV_LOG_WARNING, "Unknown projection type\n"); - goto end; } if (spherical->yaw) - put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEYAW, + put_ebml_float(pb, MATROSKA_ID_VIDEOPROJECTIONPOSEYAW, (double) spherical->yaw / (1 << 16)); if (spherical->pitch) - put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEPITCH, + put_ebml_float(pb, MATROSKA_ID_VIDEOPROJECTIONPOSEPITCH, (double) spherical->pitch / (1 << 16)); if (spherical->roll) - put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEROLL, + put_ebml_float(pb, MATROSKA_ID_VIDEOPROJECTIONPOSEROLL, (double) spherical->roll / (1 << 16)); -end: - projection_size = avio_close_dyn_buf(dyn_cp, &projection_ptr); - if (projection_size) { - ebml_master projection = start_ebml_master(pb, - MATROSKA_ID_VIDEOPROJECTION, - projection_size); - avio_write(pb, projection_ptr, projection_size); - end_ebml_master(pb, projection); - } - av_freep(&projection_ptr); + end_ebml_master(pb, projection); return 0; }
The Matroska Projection master element has such a small maximum length that it can always be written with a length field of length one. So it is unnecessary to first write the element into a dynamic buffer to get the accurate length in order not to waste bytes on the length field. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavformat/matroskaenc.c | 48 +++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 27 deletions(-)