Re: [FFmpeg-devel] [PATCH 2/2 v2] avcodec/mjpegdec: postpone calling ff_get_buffer() until the SOS marker
On Thu, Apr 22, 2021 at 02:02:33PM -0300, James Almer wrote: > With JPEG-LS PAL8 samples, the JPEG-LS extension parameters signaled with > the LSE marker show up after SOF but before SOS. For those, the pixel format > chosen by get_format() in SOF is GRAY8, and then replaced by PAL8 in LSE. > This has not been an issue given both pixel formats allocate the second data > plane for the palette, but after the upcoming soname bump, GRAY8 will no > longer > do that. This will result in segfauls when ff_jpegls_decode_lse() attempts to > write the palette on a buffer originally allocated as a GRAY8 one. > > Work around this by calling ff_get_buffer() after the actual pixel format is > known. What if the LSE occurs after the SOS ? What if there are 2 LSE ? It seems allowed by the specification "The LSE marker segment may be present where tables or miscellaneous marker segments may appear. If tables specified in this marker segment for a given table ID appear more than once, each specification shall replace the previous specification." Maybe iam missing something but a implemenattion of this would seem to require scanning through the image, finding all LSE and checking if they all are compatible with the PAL8 format before allocating the image in SOF The implemenattion here which delays allocation slightly seems to make the code much more unstable allowing the frame configuration or allocation to be partly done, double done, redone, without the matching partner. Also it does not seem to implement the related specification completely. A complete implemenattion may reqire RGB24 or 48 to be selected in some cases like when each scan uses its own palette, which unless iam missing something seems allowed. But then maybe iam missing something, in which case please correct me! thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB It is what and why we do it that matters, not just one of them. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] Revert "avformat/hlsenc: compute video_keyframe_size after write keyframe"
This reverts commit b5ca8f2c66954614d81579082025f580efc0cffc.
This commit will make new problem about tickets: 9193,9205
It flush data into file with init file context together,
and it can get keyframe size, maybe need more method to get keyframe
size.
Signed-off-by: Steven Liu
---
libavformat/hlsenc.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 7d97ce1789..e222b70ffa 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -2672,14 +2672,13 @@ static int hls_write_packet(AVFormatContext *s,
AVPacket *pkt)
vs->packets_written++;
if (oc->pb) {
-int64_t keyframe_pre_pos = avio_tell(oc->pb);
ret = ff_write_chained(oc, stream_index, pkt, s, 0);
-if ((st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) &&
-(pkt->flags & AV_PKT_FLAG_KEY) && !keyframe_pre_pos) {
-av_write_frame(oc, NULL); /* Flush any buffered data */
-vs->video_keyframe_size = avio_tell(oc->pb) - keyframe_pre_pos;
+vs->video_keyframe_size += pkt->size;
+if ((st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) && (pkt->flags &
AV_PKT_FLAG_KEY)) {
+vs->video_keyframe_size = avio_tell(oc->pb);
+} else {
+vs->video_keyframe_pos = avio_tell(vs->out);
}
-vs->video_keyframe_pos = vs->start_pos;
if (hls->ignore_io_errors)
ret = 0;
}
--
2.25.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2 v2] avcodec/mjpegdec: postpone calling ff_get_buffer() until the SOS marker
On 5/1/2021 4:01 AM, Michael Niedermayer wrote: On Thu, Apr 22, 2021 at 02:02:33PM -0300, James Almer wrote: With JPEG-LS PAL8 samples, the JPEG-LS extension parameters signaled with the LSE marker show up after SOF but before SOS. For those, the pixel format chosen by get_format() in SOF is GRAY8, and then replaced by PAL8 in LSE. This has not been an issue given both pixel formats allocate the second data plane for the palette, but after the upcoming soname bump, GRAY8 will no longer do that. This will result in segfauls when ff_jpegls_decode_lse() attempts to write the palette on a buffer originally allocated as a GRAY8 one. Work around this by calling ff_get_buffer() after the actual pixel format is known. What if the LSE occurs after the SOS ? What if there are 2 LSE ? It seems allowed by the specification "The LSE marker segment may be present where tables or miscellaneous marker segments may appear. If tables specified in this marker segment for a given table ID appear more than once, each specification shall replace the previous specification." Maybe iam missing something but a implemenattion of this would seem to require scanning through the image, finding all LSE and checking if they all are compatible with the PAL8 format before allocating the image in SOF The implemenattion here which delays allocation slightly seems to make the code much more unstable allowing the frame configuration or allocation to be partly done, double done, redone, without the matching partner. Also it does not seem to implement the related specification completely. Well, it was a hack to replace a previous hack (allocate a gray buffer then silently treat it as PAL8 once an LSE showed up). It's no wonder it may not perfectly follow the spec and consider all potential cases. A complete implemenattion may reqire RGB24 or 48 to be selected in some cases like when each scan uses its own palette, which unless iam missing something seems allowed. But then maybe iam missing something, in which case please correct me! You're probably not wrong, since unlike me you actually know this code and format. I tried to workaround an issue and it seemed to work and not break any file anyone here could test. If you think it's not good enough then just revert it and maybe allocate the data[1] pointer for the palette with av_buffer_alloc(), get_buffer2() custom implementations be damned. But i don't think the previous code even did half the things you listed above. thx [...] ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] ffprobe: add option to control optional fields display
---
doc/ffprobe.texi | 6 ++
fftools/ffprobe.c | 23 +--
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/doc/ffprobe.texi b/doc/ffprobe.texi
index d7fab4ff40..59a397a225 100644
--- a/doc/ffprobe.texi
+++ b/doc/ffprobe.texi
@@ -335,6 +335,12 @@ Show information about all pixel formats supported by
FFmpeg.
Pixel format information for each format is printed within a section
with name "PIXEL_FORMAT".
+@item -show_optional_fields @var{value}
+Some writers viz. JSON and XML, omit the printing of fields with invalid or
non-applicable values,
+while other writers always print them. This option enables one to control this
behaviour.
+Valid values are @code{always}/@code{1}, @code{never}/@code{0} and
@code{auto}/@code{-1}.
+Default is @var{auto}.
+
@item -bitexact
Force bitexact output, useful to produce output which is not dependent
on the specific build.
diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 7b28f6b3ce..b07032bd88 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -117,6 +117,11 @@ static int use_byte_value_binary_prefix = 0;
static int use_value_sexagesimal_format = 0;
static int show_private_data= 1;
+#define SHOW_OPTIONAL_FIELDS_AUTO -1
+#define SHOW_OPTIONAL_FIELDS_NEVER 0
+#define SHOW_OPTIONAL_FIELDS_ALWAYS 1
+static int show_optional_fields = SHOW_OPTIONAL_FIELDS_AUTO;
+
static char *print_format;
static char *stream_specifier;
static char *show_data_hash;
@@ -745,8 +750,10 @@ static inline int writer_print_string(WriterContext *wctx,
const struct section *section = wctx->section[wctx->level];
int ret = 0;
-if ((flags & PRINT_STRING_OPT)
-&& !(wctx->writer->flags & WRITER_FLAG_DISPLAY_OPTIONAL_FIELDS))
+if (show_optional_fields == SHOW_OPTIONAL_FIELDS_NEVER ||
+(show_optional_fields == SHOW_OPTIONAL_FIELDS_AUTO
+&& (flags & PRINT_STRING_OPT)
+&& !(wctx->writer->flags & WRITER_FLAG_DISPLAY_OPTIONAL_FIELDS)))
return 0;
if (section->show_all_entries || av_dict_get(section->entries_to_show,
key, NULL, 0)) {
@@ -3244,6 +3251,17 @@ static void ffprobe_show_pixel_formats(WriterContext *w)
writer_print_section_footer(w);
}
+static int opt_show_optional_fields(void *optctx, const char *opt, const char
*arg)
+{
+if (!av_strcasecmp(arg, "always")) show_optional_fields =
SHOW_OPTIONAL_FIELDS_ALWAYS;
+else if (!av_strcasecmp(arg, "never")) show_optional_fields =
SHOW_OPTIONAL_FIELDS_NEVER;
+else if (!av_strcasecmp(arg, "auto")) show_optional_fields =
SHOW_OPTIONAL_FIELDS_AUTO;
+
+if (show_optional_fields == SHOW_OPTIONAL_FIELDS_AUTO &&
av_strcasecmp(arg, "auto"))
+show_optional_fields = parse_number_or_die("show_optional_fields",
arg, OPT_INT, SHOW_OPTIONAL_FIELDS_AUTO, SHOW_OPTIONAL_FIELDS_ALWAYS);
+return 0;
+}
+
static int opt_format(void *optctx, const char *opt, const char *arg)
{
iformat = av_find_input_format(arg);
@@ -3631,6 +3649,7 @@ static const OptionDef real_options[] = {
{ "show_library_versions", 0, { .func_arg = &opt_show_library_versions },
"show library versions" },
{ "show_versions", 0, { .func_arg = &opt_show_versions }, "show
program and library versions" },
{ "show_pixel_formats", 0, { .func_arg = &opt_show_pixel_formats }, "show
pixel format descriptions" },
+{ "show_optional_fields", HAS_ARG, { .func_arg = &opt_show_optional_fields
}, "show optional fields" },
{ "show_private_data", OPT_BOOL, { &show_private_data }, "show private
data" },
{ "private", OPT_BOOL, { &show_private_data }, "same as
show_private_data" },
{ "bitexact", OPT_BOOL, {&do_bitexact}, "force bitexact output" },
--
2.30.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2 v2] avcodec/mjpegdec: postpone calling ff_get_buffer() until the SOS marker
On Sat, May 01, 2021 at 09:41:55AM -0300, James Almer wrote: > On 5/1/2021 4:01 AM, Michael Niedermayer wrote: > > On Thu, Apr 22, 2021 at 02:02:33PM -0300, James Almer wrote: > > > With JPEG-LS PAL8 samples, the JPEG-LS extension parameters signaled with > > > the LSE marker show up after SOF but before SOS. For those, the pixel > > > format > > > chosen by get_format() in SOF is GRAY8, and then replaced by PAL8 in LSE. > > > This has not been an issue given both pixel formats allocate the second > > > data > > > plane for the palette, but after the upcoming soname bump, GRAY8 will no > > > longer > > > do that. This will result in segfauls when ff_jpegls_decode_lse() > > > attempts to > > > write the palette on a buffer originally allocated as a GRAY8 one. > > > > > > Work around this by calling ff_get_buffer() after the actual pixel format > > > is > > > known. > > > > What if the LSE occurs after the SOS ? > > What if there are 2 LSE ? > > It seems allowed by the specification > > > > "The LSE marker segment may be present where tables or miscellaneous marker > > segments may appear. If tables specified > > in this marker segment for a given table ID appear more than once, each > > specification shall replace the previous > > specification." > > > > Maybe iam missing something but a implemenattion of this would seem to > > require scanning through the image, finding all LSE and checking if they all > > are compatible with the PAL8 format before allocating the image in SOF > > > > The implemenattion here which delays allocation slightly seems to make > > the code much more unstable allowing the frame configuration or allocation > > to be partly done, double done, redone, without the matching partner. > > Also it does not seem to implement the related specification completely. > > Well, it was a hack to replace a previous hack (allocate a gray buffer then > silently treat it as PAL8 once an LSE showed up). It's no wonder it may not > perfectly follow the spec and consider all potential cases. yes and i wouldnt mind but the new hack is leading to crashes ... and the fixes to the crashes all in all start looking a bit ugly and iam not sure if any more remain. So id like to take a bit a step back here and see if there isnt a easier/cleaner solution. > > > > > A complete implemenattion may reqire RGB24 or 48 to be selected in some > > cases > > like when each scan uses its own palette, which unless iam missing something > > seems allowed. > > But then maybe iam missing something, in which case please correct me! > > You're probably not wrong, since unlike me you actually know this code and > format. I tried to workaround an issue and it seemed to work and not break > any file anyone here could test. > > If you think it's not good enough then just revert it and maybe allocate the > data[1] pointer for the palette with av_buffer_alloc(), get_buffer2() custom > implementations be damned. But i don't think the previous code even did half > the things you listed above. how can i reproduce the issue you where trying to fix with this patch ? thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have never wished to cater to the crowd; for what I know they do not approve, and what they approve I do not know. -- Epicurus signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2 v2] avcodec/mjpegdec: postpone calling ff_get_buffer() until the SOS marker
On 5/1/2021 11:17 AM, Michael Niedermayer wrote: On Sat, May 01, 2021 at 09:41:55AM -0300, James Almer wrote: On 5/1/2021 4:01 AM, Michael Niedermayer wrote: On Thu, Apr 22, 2021 at 02:02:33PM -0300, James Almer wrote: With JPEG-LS PAL8 samples, the JPEG-LS extension parameters signaled with the LSE marker show up after SOF but before SOS. For those, the pixel format chosen by get_format() in SOF is GRAY8, and then replaced by PAL8 in LSE. This has not been an issue given both pixel formats allocate the second data plane for the palette, but after the upcoming soname bump, GRAY8 will no longer do that. This will result in segfauls when ff_jpegls_decode_lse() attempts to write the palette on a buffer originally allocated as a GRAY8 one. Work around this by calling ff_get_buffer() after the actual pixel format is known. What if the LSE occurs after the SOS ? What if there are 2 LSE ? It seems allowed by the specification "The LSE marker segment may be present where tables or miscellaneous marker segments may appear. If tables specified in this marker segment for a given table ID appear more than once, each specification shall replace the previous specification." Maybe iam missing something but a implemenattion of this would seem to require scanning through the image, finding all LSE and checking if they all are compatible with the PAL8 format before allocating the image in SOF The implemenattion here which delays allocation slightly seems to make the code much more unstable allowing the frame configuration or allocation to be partly done, double done, redone, without the matching partner. Also it does not seem to implement the related specification completely. Well, it was a hack to replace a previous hack (allocate a gray buffer then silently treat it as PAL8 once an LSE showed up). It's no wonder it may not perfectly follow the spec and consider all potential cases. yes and i wouldnt mind but the new hack is leading to crashes ... and the fixes to the crashes all in all start looking a bit ugly and iam not sure if any more remain. So id like to take a bit a step back here and see if there isnt a easier/cleaner solution. A complete implemenattion may reqire RGB24 or 48 to be selected in some cases like when each scan uses its own palette, which unless iam missing something seems allowed. But then maybe iam missing something, in which case please correct me! You're probably not wrong, since unlike me you actually know this code and format. I tried to workaround an issue and it seemed to work and not break any file anyone here could test. If you think it's not good enough then just revert it and maybe allocate the data[1] pointer for the palette with av_buffer_alloc(), get_buffer2() custom implementations be damned. But i don't think the previous code even did half the things you listed above. how can i reproduce the issue you where trying to fix with this patch ? It's the crash you reported in https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2021-April/279350.html The explanation of why it started crashing is in c8197f73e6. Basically, since GRAY8 no longer unnecessarily allocates a fixed palette, the old hack of changing pix_fmt from GRAY8 to PAL8 after having called ff_get_buffer() with the former, editing the palette plane buffer, and have things magically work is not possible anymore, because no palette buffer was ever allocated. ff_get_buffer() is meant to be called once the actual pix_fmt is known to correctly allocate all plane buffers. thx [...] ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2 v2] avcodec/mjpegdec: postpone calling ff_get_buffer() until the SOS marker
On 5/1/2021 11:30 AM, James Almer wrote: On 5/1/2021 11:17 AM, Michael Niedermayer wrote: On Sat, May 01, 2021 at 09:41:55AM -0300, James Almer wrote: On 5/1/2021 4:01 AM, Michael Niedermayer wrote: On Thu, Apr 22, 2021 at 02:02:33PM -0300, James Almer wrote: With JPEG-LS PAL8 samples, the JPEG-LS extension parameters signaled with the LSE marker show up after SOF but before SOS. For those, the pixel format chosen by get_format() in SOF is GRAY8, and then replaced by PAL8 in LSE. This has not been an issue given both pixel formats allocate the second data plane for the palette, but after the upcoming soname bump, GRAY8 will no longer do that. This will result in segfauls when ff_jpegls_decode_lse() attempts to write the palette on a buffer originally allocated as a GRAY8 one. Work around this by calling ff_get_buffer() after the actual pixel format is known. What if the LSE occurs after the SOS ? What if there are 2 LSE ? It seems allowed by the specification "The LSE marker segment may be present where tables or miscellaneous marker segments may appear. If tables specified in this marker segment for a given table ID appear more than once, each specification shall replace the previous specification." Maybe iam missing something but a implemenattion of this would seem to require scanning through the image, finding all LSE and checking if they all are compatible with the PAL8 format before allocating the image in SOF The implemenattion here which delays allocation slightly seems to make the code much more unstable allowing the frame configuration or allocation to be partly done, double done, redone, without the matching partner. Also it does not seem to implement the related specification completely. Well, it was a hack to replace a previous hack (allocate a gray buffer then silently treat it as PAL8 once an LSE showed up). It's no wonder it may not perfectly follow the spec and consider all potential cases. yes and i wouldnt mind but the new hack is leading to crashes ... and the fixes to the crashes all in all start looking a bit ugly and iam not sure if any more remain. So id like to take a bit a step back here and see if there isnt a easier/cleaner solution. A complete implemenattion may reqire RGB24 or 48 to be selected in some cases like when each scan uses its own palette, which unless iam missing something seems allowed. But then maybe iam missing something, in which case please correct me! You're probably not wrong, since unlike me you actually know this code and format. I tried to workaround an issue and it seemed to work and not break any file anyone here could test. If you think it's not good enough then just revert it and maybe allocate the data[1] pointer for the palette with av_buffer_alloc(), get_buffer2() custom implementations be damned. But i don't think the previous code even did half the things you listed above. how can i reproduce the issue you where trying to fix with this patch ? It's the crash you reported in https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2021-April/279350.html The explanation of why it started crashing is in c8197f73e6. Basically, since GRAY8 no longer unnecessarily allocates a fixed palette, the old hack of changing pix_fmt from GRAY8 to PAL8 after having called ff_get_buffer() with the former, editing the palette plane buffer, and have things magically work is not possible anymore, because no palette buffer was ever allocated. ff_get_buffer() is meant to be called once the actual pix_fmt is known to correctly allocate all plane buffers. Another possible solution instead of my suggestion above (revert and then manually allocate a palette plane buffer, which can potentially piss off custom get_buffer2() implementations), is to revert and then initially set the pix_fmt in ff_mjpeg_decode_sof() to PAL8 instead of GRAY8 for jpegls, and replace it with the latter if no LSE is ever parsed (or if one is parsed and it doesn't contain a palette). It's also hacky, basically the inverse of what it used to be, but at least it ensures the palette plane is allocated by get_buffer2(), which will ultimately be ignored and freed. thx [...] ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2 v2] avcodec/mjpegdec: postpone calling ff_get_buffer() until the SOS marker
On 5/1/2021 12:06 PM, James Almer wrote:
On 5/1/2021 11:30 AM, James Almer wrote:
On 5/1/2021 11:17 AM, Michael Niedermayer wrote:
On Sat, May 01, 2021 at 09:41:55AM -0300, James Almer wrote:
On 5/1/2021 4:01 AM, Michael Niedermayer wrote:
On Thu, Apr 22, 2021 at 02:02:33PM -0300, James Almer wrote:
With JPEG-LS PAL8 samples, the JPEG-LS extension parameters
signaled with
the LSE marker show up after SOF but before SOS. For those, the
pixel format
chosen by get_format() in SOF is GRAY8, and then replaced by PAL8
in LSE.
This has not been an issue given both pixel formats allocate the
second data
plane for the palette, but after the upcoming soname bump, GRAY8
will no longer
do that. This will result in segfauls when ff_jpegls_decode_lse()
attempts to
write the palette on a buffer originally allocated as a GRAY8 one.
Work around this by calling ff_get_buffer() after the actual pixel
format is
known.
What if the LSE occurs after the SOS ?
What if there are 2 LSE ?
It seems allowed by the specification
"The LSE marker segment may be present where tables or
miscellaneous marker segments may appear. If tables specified
in this marker segment for a given table ID appear more than
once, each specification shall replace the previous
specification."
Maybe iam missing something but a implemenattion of this would seem to
require scanning through the image, finding all LSE and checking if
they all
are compatible with the PAL8 format before allocating the image in SOF
The implemenattion here which delays allocation slightly seems to make
the code much more unstable allowing the frame configuration or
allocation
to be partly done, double done, redone, without the matching partner.
Also it does not seem to implement the related specification
completely.
Well, it was a hack to replace a previous hack (allocate a gray
buffer then
silently treat it as PAL8 once an LSE showed up). It's no wonder it
may not
perfectly follow the spec and consider all potential cases.
yes and i wouldnt mind but the new hack is leading to crashes ...
and the fixes to the crashes all in all start looking a bit ugly and
iam not
sure if any more remain. So id like to take a bit a step back here
and see
if there isnt a easier/cleaner solution.
A complete implemenattion may reqire RGB24 or 48 to be selected in
some cases
like when each scan uses its own palette, which unless iam missing
something
seems allowed.
But then maybe iam missing something, in which case please correct me!
You're probably not wrong, since unlike me you actually know this
code and
format. I tried to workaround an issue and it seemed to work and not
break
any file anyone here could test.
If you think it's not good enough then just revert it and maybe
allocate the
data[1] pointer for the palette with av_buffer_alloc(),
get_buffer2() custom
implementations be damned. But i don't think the previous code even
did half
the things you listed above.
how can i reproduce the issue you where trying to fix with this patch ?
It's the crash you reported in
https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2021-April/279350.html
The explanation of why it started crashing is in c8197f73e6.
Basically, since GRAY8 no longer unnecessarily allocates a fixed
palette, the old hack of changing pix_fmt from GRAY8 to PAL8 after
having called ff_get_buffer() with the former, editing the palette
plane buffer, and have things magically work is not possible anymore,
because no palette buffer was ever allocated. ff_get_buffer() is meant
to be called once the actual pix_fmt is known to correctly allocate
all plane buffers.
Another possible solution instead of my suggestion above (revert and
then manually allocate a palette plane buffer, which can potentially
piss off custom get_buffer2() implementations), is to revert and then
initially set the pix_fmt in ff_mjpeg_decode_sof() to PAL8 instead of
GRAY8 for jpegls, and replace it with the latter if no LSE is ever
parsed (or if one is parsed and it doesn't contain a palette).
It's also hacky, basically the inverse of what it used to be, but at
least it ensures the palette plane is allocated by get_buffer2(), which
will ultimately be ignored and freed.
Which is to say, revert fb5e2d7112 c8197f73e6 and try the something like
the following:
$ git diff
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 7c66ff8637..70e674758d 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -681,10 +681,8 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
} else if (s->nb_components != 1) {
av_log(s->avctx, AV_LOG_ERROR, "Unsupported number of components
%d\n", s->nb_components);
return AVERROR_PATCHWELCOME;
-} else if (s->palette_index && s->bits <= 8)
+} else if (s->bits <= 8)
s->avctx->pix_fmt = AV_PIX_FMT_PAL8;
-else if (s->bits <= 8)
-s->avct
[FFmpeg-devel] [PATCH] lavfi/dnn_backend_native_layer_avgpool.c: Correct Spelling of Pixel
Correct spelling of word `pixel` from `pxiels` Signed-off-by: Shubhanshu Saxena --- libavfilter/dnn/dnn_backend_native_layer_avgpool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavfilter/dnn/dnn_backend_native_layer_avgpool.c b/libavfilter/dnn/dnn_backend_native_layer_avgpool.c index dcfb8c816f..89f1787523 100644 --- a/libavfilter/dnn/dnn_backend_native_layer_avgpool.c +++ b/libavfilter/dnn/dnn_backend_native_layer_avgpool.c @@ -73,7 +73,7 @@ int ff_dnn_execute_layer_avg_pool(DnnOperand *operands, const int32_t *input_ope DnnOperand *output_operand = &operands[output_operand_index]; /** - * When padding_method = SAME, the tensorflow will only padding the hald number of 0 pxiels + * When padding_method = SAME, the tensorflow will only padding the hald number of 0 pixels * except the remainders. * Eg: assuming the input height = 1080, the strides = 11, so the remainders = 1080 % 11 = 2 * and if ksize = 5: it will fill (5 - 2) >> 1 = 1 line before the first line of input image, -- 2.27.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] avformat/dv: use av_packet_alloc() to allocate packets
James Almer:
> As avpriv_dv_get_packet can fail now, make it return < 0 on error, 0 on no
> packet found, and > 0 on packet found.
>
> Signed-off-by: James Almer
> ---
> libavdevice/iec61883.c | 2 +-
> libavformat/avidec.c | 4 +++-
> libavformat/dv.c | 51 ++
> 3 files changed, 36 insertions(+), 21 deletions(-)
>
> diff --git a/libavdevice/iec61883.c b/libavdevice/iec61883.c
> index 18ad704066..de9f48b8fc 100644
> --- a/libavdevice/iec61883.c
> +++ b/libavdevice/iec61883.c
> @@ -191,7 +191,7 @@ static int iec61883_parse_queue_dv(struct iec61883_data
> *dv, AVPacket *pkt)
> int size;
>
> size = avpriv_dv_get_packet(dv->dv_demux, pkt);
> -if (size > 0)
> +if (size)
> return size;
>
> packet = dv->queue_first;
> diff --git a/libavformat/avidec.c b/libavformat/avidec.c
> index 2d0d2a7389..2f493e42a6 100644
> --- a/libavformat/avidec.c
> +++ b/libavformat/avidec.c
> @@ -1440,8 +1440,10 @@ static int avi_read_packet(AVFormatContext *s,
> AVPacket *pkt)
>
> if (CONFIG_DV_DEMUXER && avi->dv_demux) {
> int size = avpriv_dv_get_packet(avi->dv_demux, pkt);
> -if (size >= 0)
> +if (size > 0)
> return size;
> +else if (size < 0)
> +return AVERROR(ENOMEM);
> else
> goto resync;
> }
> diff --git a/libavformat/dv.c b/libavformat/dv.c
> index a948fc0b98..1adc9fdb7b 100644
> --- a/libavformat/dv.c
> +++ b/libavformat/dv.c
> @@ -45,7 +45,7 @@ struct DVDemuxContext {
> AVFormatContext* fctx;
> AVStream* vst;
> AVStream* ast[4];
> -AVPacket audio_pkt[4];
> +AVPacket *audio_pkt[4];
> uint8_t audio_buf[4][8192];
> int ach;
> int frames;
> @@ -261,11 +261,11 @@ static int dv_extract_audio_info(DVDemuxContext *c,
> const uint8_t *frame)
> c->ast[i]->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
> c->ast[i]->codecpar->codec_id = AV_CODEC_ID_PCM_S16LE;
>
> -av_init_packet(&c->audio_pkt[i]);
> -c->audio_pkt[i].size = 0;
> -c->audio_pkt[i].data = c->audio_buf[i];
> -c->audio_pkt[i].stream_index = c->ast[i]->index;
> -c->audio_pkt[i].flags |= AV_PKT_FLAG_KEY;
> +av_packet_unref(c->audio_pkt[i]);
> +c->audio_pkt[i]->size = 0;
> +c->audio_pkt[i]->data = c->audio_buf[i];
> +c->audio_pkt[i]->stream_index = c->ast[i]->index;
> +c->audio_pkt[i]->flags |= AV_PKT_FLAG_KEY;
> }
> c->ast[i]->codecpar->sample_rate= dv_audio_frequency[freq];
> c->ast[i]->codecpar->channels = 2;
> @@ -327,6 +327,9 @@ void avpriv_dv_close_demux(DVDemuxContext **pc)
> if (!c)
> return;
>
> +for (int i = 0; i < 4; i++)
> +av_packet_free(&c->audio_pkt[i]);
> +
> av_freep(pc);
> }
>
> @@ -336,6 +339,12 @@ static int dv_init_demux(AVFormatContext *s,
> DVDemuxContext *c)
> if (!c->vst)
> return AVERROR(ENOMEM);
>
> +for (int i = 0; i < 4; i++) {
> +c->audio_pkt[i] = av_packet_alloc();
> +if (!c->audio_pkt[i])
> + return AVERROR(ENOMEM);
> +}
> +
> c->fctx = s;
> c->vst->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> c->vst->codecpar->codec_id = AV_CODEC_ID_DVVIDEO;
> @@ -361,13 +370,14 @@ DVDemuxContext *avpriv_dv_init_demux(AVFormatContext *s)
>
> int avpriv_dv_get_packet(DVDemuxContext *c, AVPacket *pkt)
> {
> -int size = -1;
> +int size = 0;
> int i;
>
> for (i = 0; i < c->ach; i++) {
> -if (c->ast[i] && c->audio_pkt[i].size) {
> -*pkt = c->audio_pkt[i];
> -c->audio_pkt[i].size = 0;
> +if (c->ast[i] && c->audio_pkt[i]->size) {
> +if (av_packet_ref(pkt, c->audio_pkt[i]) < 0)
> +return -1;
> +c->audio_pkt[i]->size = 0;
> size = pkt->size;
> break;
> }
> @@ -392,9 +402,9 @@ int avpriv_dv_produce_packet(DVDemuxContext *c, AVPacket
> *pkt,
> /* FIXME: in case of no audio/bad audio we have to do something */
> size = dv_extract_audio_info(c, buf);
> for (i = 0; i < c->ach; i++) {
> -c->audio_pkt[i].pos = pos;
> -c->audio_pkt[i].size = size;
> -c->audio_pkt[i].pts = (c->sys->height == 720) ? (c->frames & ~1) :
> c->frames;
> +c->audio_pkt[i]->pos = pos;
> +c->audio_pkt[i]->size = size;
> +c->audio_pkt[i]->pts = (c->sys->height == 720) ? (c->frames & ~1) :
> c->frames;
> ppcm[i] = c->audio_buf[i];
> }
> if (c->ach)
> @@ -404,15 +414,15 @@ int avpriv_dv_produce_packet(DVDemuxContext *c,
> AVPacket *pkt,
> * channels 0,1 and odd 2,3. */
> if (c->sys->height == 720) {
>
Re: [FFmpeg-devel] [PATCH 01/46] avcodec/a64multienc: Avoid intermediate buffer
Andreas Rheinhardt:
> Signed-off-by: Andreas Rheinhardt
> ---
> This patchset is the first batch of patches to implement support for
> user-supplied buffers in encoders; those who can't wait can already look
> here for the second part dealing with encoders with external libs:
> https://github.com/mkver/FFmpeg/commits/ff_alloc_packet2
> This patchset supersedes
> https://ffmpeg.org/pipermail/ffmpeg-devel/2021-April/279571.html
>
> libavcodec/a64multienc.c | 11 ++-
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/libavcodec/a64multienc.c b/libavcodec/a64multienc.c
> index 9dc859b271..1b52631193 100644
> --- a/libavcodec/a64multienc.c
> +++ b/libavcodec/a64multienc.c
> @@ -50,7 +50,6 @@ typedef struct A64Context {
> int *mc_charmap;
> int *mc_best_cb;
> int mc_luma_vals[5];
> -uint8_t *mc_charset;
> uint8_t *mc_colram;
> uint8_t *mc_palette;
> int mc_pal_size;
> @@ -197,7 +196,6 @@ static av_cold int a64multi_close_encoder(AVCodecContext
> *avctx)
> A64Context *c = avctx->priv_data;
> av_freep(&c->mc_meta_charset);
> av_freep(&c->mc_best_cb);
> -av_freep(&c->mc_charset);
> av_freep(&c->mc_charmap);
> av_freep(&c->mc_colram);
> return 0;
> @@ -231,8 +229,7 @@ static av_cold int a64multi_encode_init(AVCodecContext
> *avctx)
> if (!(c->mc_meta_charset = av_mallocz_array(c->mc_lifetime, 32000 *
> sizeof(int))) ||
> !(c->mc_best_cb = av_malloc(CHARSET_CHARS * 32 * sizeof(int)))
>||
> !(c->mc_charmap = av_mallocz_array(c->mc_lifetime, 1000 *
> sizeof(int))) ||
> - !(c->mc_colram= av_mallocz(CHARSET_CHARS * sizeof(uint8_t)))
>||
> - !(c->mc_charset = av_malloc(0x800 * (INTERLACED+1) *
> sizeof(uint8_t {
> + !(c->mc_colram= av_mallocz(CHARSET_CHARS * sizeof(uint8_t
> {
> av_log(avctx, AV_LOG_ERROR, "Failed to allocate buffer memory.\n");
> return AVERROR(ENOMEM);
> }
> @@ -284,7 +281,6 @@ static int a64multi_encode_frame(AVCodecContext *avctx,
> AVPacket *pkt,
>
> int *charmap = c->mc_charmap;
> uint8_t *colram = c->mc_colram;
> -uint8_t *charset = c->mc_charset;
> int *meta= c->mc_meta_charset;
> int *best_cb = c->mc_best_cb;
>
> @@ -346,10 +342,7 @@ static int a64multi_encode_frame(AVCodecContext *avctx,
> AVPacket *pkt,
> return ret;
>
> /* create colorram map and a c64 readable charset */
> -render_charset(avctx, charset, colram);
> -
> -/* copy charset to buf */
> -memcpy(buf, charset, charset_size);
> +render_charset(avctx, buf, colram);
>
> /* advance pointers */
> buf += charset_size;
>
Will apply this patchset tomorrow unless there are objections.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
