On Mon, Sep 17, 2018 at 02:35:44PM -0700, Jacob Trimble wrote: > On Wed, Sep 12, 2018 at 11:50 AM Michael Niedermayer > <mich...@niedermayer.cc> wrote: > > > > On Tue, Sep 11, 2018 at 03:50:57PM -0700, Jacob Trimble wrote: > > > [...] > > > > > > So how about, when we see an encrypted frame, we flush the parser > > > before skipping the frame? Can we just flush the parser and then > > > reuse it later? Or would we need to create a new parser if we saw > > > clear frames later? > > > > try/test it and review the code. only way to know for sure what works > > > > thanks > > > > [...] > > -- > > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > > > The bravest are surely those who have the clearest vision > > of what is before them, glory and danger alike, and yet > > notwithstanding go out to meet it. -- Thucydides > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > Here is a new patch that flushes the parser before returning the > encrypted packets. It also logs when it does this.
> utils.c | 41 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 40 insertions(+), 1 deletion(-)
> ef62a54a0acee61029b78e294a030f416785b64a
> 0001-avformat-utils-Don-t-parse-encrypted-packets-v2.patch
> From 3955dad8070c28b021afc3304939500a09c86fcd Mon Sep 17 00:00:00 2001
> From: Jacob Trimble <modma...@google.com>
> Date: Tue, 28 Aug 2018 10:40:29 -0700
> Subject: [PATCH] avformat/utils: Don't parse encrypted packets.
>
> If a packet is full-sample encrypted, then packet data can't be parsed
> without decrypting it. So this skips the packet parsing for those
> packets. If the packet has sub-sample encryption, it is assumed that
> the headers are in the clear and the parser will only need that info.
>
> Signed-off-by: Jacob Trimble <modma...@google.com>
> ---
> libavformat/utils.c | 41 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index a72f0a482e..8d84674797 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -27,6 +27,7 @@
> #include "libavutil/avassert.h"
> #include "libavutil/avstring.h"
> #include "libavutil/dict.h"
> +#include "libavutil/encryption_info.h"
> #include "libavutil/internal.h"
> #include "libavutil/mathematics.h"
> #include "libavutil/opt.h"
> @@ -1576,6 +1577,9 @@ static int read_frame_internal(AVFormatContext *s,
> AVPacket *pkt)
> while (!got_packet && !s->internal->parse_queue) {
> AVStream *st;
> AVPacket cur_pkt;
> + uint8_t *enc_side_data;
> + int enc_side_data_size;
> + int is_full_encrypted = 0;
>
> /* read next packet */
> ret = ff_read_packet(s, &cur_pkt);
> @@ -1643,7 +1647,23 @@ FF_ENABLE_DEPRECATION_WARNINGS
> av_ts2str(cur_pkt.dts),
> cur_pkt.size, cur_pkt.duration, cur_pkt.flags);
>
> - if (st->need_parsing && !st->parser && !(s->flags &
> AVFMT_FLAG_NOPARSE)) {
> + /* if the packet is full-sample encrypted, we can't parse it. If the
> + * packet uses sub-sample encryption, assume the headers are clear
> and
> + * can still be parsed.
> + */
> + enc_side_data = av_packet_get_side_data(
> + &cur_pkt, AV_PKT_DATA_ENCRYPTION_INFO, &enc_side_data_size);
> + if (enc_side_data) {
> + AVEncryptionInfo *enc_info =
> + av_encryption_info_get_side_data(enc_side_data,
> enc_side_data_size);
> + if (enc_info) {
> + is_full_encrypted = enc_info->subsample_count == 0;
> + av_encryption_info_free(enc_info);
> + }
> + }
I dont think that a allocation and deallocation for basically checking one bit
is ideal. This is done for every packet and there can be many small packets.
> +
> + if (st->need_parsing && !st->parser && !(s->flags &
> AVFMT_FLAG_NOPARSE) &&
> + !is_full_encrypted) {
> st->parser = av_parser_init(st->codecpar->codec_id);
> if (!st->parser) {
> av_log(s, AV_LOG_VERBOSE, "parser not found for codec "
> @@ -1659,6 +1679,25 @@ FF_ENABLE_DEPRECATION_WARNINGS
> st->parser->flags |= PARSER_FLAG_USE_CODEC_TS;
> }
>
> + if (st->parser && is_full_encrypted) {
> + av_log(s, AV_LOG_VERBOSE, "skipping parsing of encrypted
> packets.\n");
> +
> + /* flush any packets from the parser. */
> + parse_packet(s, NULL, cur_pkt.stream_index);
> + if (s->internal->parse_queue) {
> + /* if we have other packets, append this packet to the end
> and read
> + * from the queue instead. */
> + compute_pkt_fields(s, st, NULL, &cur_pkt, AV_NOPTS_VALUE,
> AV_NOPTS_VALUE);
> + ret = ff_packet_list_put(&s->internal->parse_queue,
> + &s->internal->parse_queue_end,
> + &cur_pkt, 0);
> + if (ret < 0)
> + return ret;
> + break;
> + }
> + av_assert2(!st->parser);
> + }
have you tested streams that mix encrypted and unencrypted packets?
I mean has this parser on/off code been tested ?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
