diff mbox

[FFmpeg-devel,14/37] avformat/matroskadec: Remove non-incremental parsing of clusters

Message ID 20190516223018.30827-15-andreas.rheinhardt@gmail.com
State Accepted
Headers show

Commit Message

Andreas Rheinhardt May 16, 2019, 10:29 p.m. UTC 
When the new incremental parser was introduced, the old parser was
kept, because the new parser was unable to handle the way SSA packets
are put into Matroska. But since 2014 (since c7d8dbad) this is no
longer needed, so that the old parser can be completely removed.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/matroskadec.c | 72 ++++++---------------------------------
 1 file changed, 10 insertions(+), 62 deletions(-)

Comments

James Almer June 23, 2019, 3:04 a.m. UTC | #1 
On 5/16/2019 7:29 PM, Andreas Rheinhardt wrote:
> When the new incremental parser was introduced, the old parser was
> kept, because the new parser was unable to handle the way SSA packets
> are put into Matroska. But since 2014 (since c7d8dbad) this is no
> longer needed, so that the old parser can be completely removed.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavformat/matroskadec.c | 72 ++++++---------------------------------
>  1 file changed, 10 insertions(+), 62 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index e01f1703e7..c0767c3529 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -362,9 +362,6 @@ typedef struct MatroskaDemuxContext {
>      int64_t current_cluster_pos;
>      MatroskaCluster current_cluster;
>  
> -    /* File has SSA subtitles which prevent incremental cluster parsing. */
> -    int contains_ssa;
> -
>      /* WebM DASH Manifest live flag */
>      int is_live;
>  
> @@ -709,25 +706,7 @@ static const EbmlSyntax matroska_blockgroup[] = {
>      { 0 }
>  };
>  
> -static const EbmlSyntax matroska_cluster[] = {
> -    { MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0,                     offsetof(MatroskaCluster, timecode) },
> -    { MATROSKA_ID_BLOCKGROUP,      EBML_NEST, sizeof(MatroskaBlock), offsetof(MatroskaCluster, blocks), { .n = matroska_blockgroup } },
> -    { MATROSKA_ID_SIMPLEBLOCK,     EBML_PASS, sizeof(MatroskaBlock), offsetof(MatroskaCluster, blocks), { .n = matroska_blockgroup } },
> -    { MATROSKA_ID_CLUSTERPOSITION, EBML_NONE },
> -    { MATROSKA_ID_CLUSTERPREVSIZE, EBML_NONE },
> -    { 0 }
> -};
> -
> -static const EbmlSyntax matroska_clusters[] = {
> -    { MATROSKA_ID_CLUSTER,  EBML_NEST, 0, 0, { .n = matroska_cluster } },
> -    { MATROSKA_ID_INFO,     EBML_NONE },
> -    { MATROSKA_ID_CUES,     EBML_NONE },
> -    { MATROSKA_ID_TAGS,     EBML_NONE },
> -    { MATROSKA_ID_SEEKHEAD, EBML_NONE },
> -    { 0 }
> -};
> -
> -static const EbmlSyntax matroska_cluster_incremental_parsing[] = {
> +static const EbmlSyntax matroska_cluster_parsing[] = {
>      { MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0,                     offsetof(MatroskaCluster, timecode) },
>      { MATROSKA_ID_BLOCKGROUP,      EBML_NEST, sizeof(MatroskaBlock), offsetof(MatroskaCluster, blocks), { .n = matroska_blockgroup } },
>      { MATROSKA_ID_SIMPLEBLOCK,     EBML_PASS, sizeof(MatroskaBlock), offsetof(MatroskaCluster, blocks), { .n = matroska_blockgroup } },
> @@ -741,7 +720,7 @@ static const EbmlSyntax matroska_cluster_incremental_parsing[] = {
>      { 0 }
>  };
>  
> -static const EbmlSyntax matroska_cluster_incremental[] = {
> +static const EbmlSyntax matroska_cluster[] = {
>      { MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0, offsetof(MatroskaCluster, timecode) },
>      { MATROSKA_ID_BLOCKGROUP,      EBML_STOP },
>      { MATROSKA_ID_SIMPLEBLOCK,     EBML_STOP },
> @@ -750,8 +729,8 @@ static const EbmlSyntax matroska_cluster_incremental[] = {
>      { 0 }
>  };
>  
> -static const EbmlSyntax matroska_clusters_incremental[] = {
> -    { MATROSKA_ID_CLUSTER,  EBML_NEST, 0, 0, { .n = matroska_cluster_incremental } },
> +static const EbmlSyntax matroska_clusters[] = {
> +    { MATROSKA_ID_CLUSTER,  EBML_NEST, 0, 0, { .n = matroska_cluster } },
>      { MATROSKA_ID_INFO,     EBML_NONE },
>      { MATROSKA_ID_CUES,     EBML_NONE },
>      { MATROSKA_ID_TAGS,     EBML_NONE },
> @@ -2648,8 +2627,6 @@ static int matroska_parse_tracks(AVFormatContext *s)
>              }
>          } else if (track->type == MATROSKA_TRACK_TYPE_SUBTITLE) {
>              st->codecpar->codec_type = AVMEDIA_TYPE_SUBTITLE;
> -            if (st->codecpar->codec_id == AV_CODEC_ID_ASS)
> -                matroska->contains_ssa = 1;
>          }
>      }
>  
> @@ -3520,19 +3497,19 @@ end:
>      return res;
>  }
>  
> -static int matroska_parse_cluster_incremental(MatroskaDemuxContext *matroska)
> +static int matroska_parse_cluster(MatroskaDemuxContext *matroska)
>  {
>      EbmlList *blocks_list;
>      MatroskaBlock *blocks;
>      int i, res;
>      res = ebml_parse(matroska,
> -                     matroska_cluster_incremental_parsing,
> +                     matroska_cluster_parsing,
>                       &matroska->current_cluster);
>      if (res == 1) {
>          /* New Cluster */
>          if (matroska->current_cluster_pos)
>              ebml_level_end(matroska);
> -        ebml_free(matroska_cluster, &matroska->current_cluster);
> +        ebml_free(matroska_cluster_parsing, &matroska->current_cluster);
>          memset(&matroska->current_cluster, 0, sizeof(MatroskaCluster));
>          matroska->current_cluster_num_blocks = 0;
>          matroska->current_cluster_pos        = avio_tell(matroska->ctx->pb);
> @@ -3540,12 +3517,12 @@ static int matroska_parse_cluster_incremental(MatroskaDemuxContext *matroska)
>          if (matroska->current_id)
>              matroska->current_cluster_pos -= 4;
>          res = ebml_parse(matroska,
> -                         matroska_clusters_incremental,
> +                         matroska_clusters,
>                           &matroska->current_cluster);
>          /* Try parsing the block again. */
>          if (res == 1)
>              res = ebml_parse(matroska,
> -                             matroska_cluster_incremental_parsing,
> +                             matroska_cluster_parsing,
>                               &matroska->current_cluster);
>      }
>  
> @@ -3576,35 +3553,6 @@ static int matroska_parse_cluster_incremental(MatroskaDemuxContext *matroska)
>      return res;
>  }
>  
> -static int matroska_parse_cluster(MatroskaDemuxContext *matroska)
> -{
> -    MatroskaCluster cluster = { 0 };
> -    EbmlList *blocks_list;
> -    MatroskaBlock *blocks;
> -    int i, res;
> -    int64_t pos;
> -
> -    if (!matroska->contains_ssa)
> -        return matroska_parse_cluster_incremental(matroska);
> -    pos = avio_tell(matroska->ctx->pb);
> -    if (matroska->current_id)
> -        pos -= 4;  /* sizeof the ID which was already read */
> -    res         = ebml_parse(matroska, matroska_clusters, &cluster);
> -    blocks_list = &cluster.blocks;
> -    blocks      = blocks_list->elem;
> -    for (i = 0; i < blocks_list->nb_elem; i++)
> -        if (blocks[i].bin.size > 0 && blocks[i].bin.data) {
> -            int is_keyframe = blocks[i].non_simple ? blocks[i].reference == INT64_MIN : -1;
> -            res = matroska_parse_block(matroska, blocks[i].bin.buf, blocks[i].bin.data,
> -                                       blocks[i].bin.size, blocks[i].bin.pos,
> -                                       cluster.timecode, blocks[i].duration,
> -                                       is_keyframe, NULL, 0, 0, pos,
> -                                       blocks[i].discard_padding);
> -        }
> -    ebml_free(matroska_cluster, &cluster);
> -    return res;
> -}
> -
>  static int matroska_read_packet(AVFormatContext *s, AVPacket *pkt)
>  {
>      MatroskaDemuxContext *matroska = s->priv_data;
> @@ -3699,7 +3647,7 @@ static int matroska_read_close(AVFormatContext *s)
>      for (n = 0; n < matroska->tracks.nb_elem; n++)
>          if (tracks[n].type == MATROSKA_TRACK_TYPE_AUDIO)
>              av_freep(&tracks[n].audio.buf);
> -    ebml_free(matroska_cluster, &matroska->current_cluster);
> +    ebml_free(matroska_cluster_parsing, &matroska->current_cluster);
>      ebml_free(matroska_segment, matroska);
>  
>      return 0;

Applied.
diff mbox

Patch

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index e01f1703e7..c0767c3529 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -362,9 +362,6 @@  typedef struct MatroskaDemuxContext {
     int64_t current_cluster_pos;
     MatroskaCluster current_cluster;
 
-    /* File has SSA subtitles which prevent incremental cluster parsing. */
-    int contains_ssa;
-
     /* WebM DASH Manifest live flag */
     int is_live;
 
@@ -709,25 +706,7 @@  static const EbmlSyntax matroska_blockgroup[] = {
     { 0 }
 };
 
-static const EbmlSyntax matroska_cluster[] = {
-    { MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0,                     offsetof(MatroskaCluster, timecode) },
-    { MATROSKA_ID_BLOCKGROUP,      EBML_NEST, sizeof(MatroskaBlock), offsetof(MatroskaCluster, blocks), { .n = matroska_blockgroup } },
-    { MATROSKA_ID_SIMPLEBLOCK,     EBML_PASS, sizeof(MatroskaBlock), offsetof(MatroskaCluster, blocks), { .n = matroska_blockgroup } },
-    { MATROSKA_ID_CLUSTERPOSITION, EBML_NONE },
-    { MATROSKA_ID_CLUSTERPREVSIZE, EBML_NONE },
-    { 0 }
-};
-
-static const EbmlSyntax matroska_clusters[] = {
-    { MATROSKA_ID_CLUSTER,  EBML_NEST, 0, 0, { .n = matroska_cluster } },
-    { MATROSKA_ID_INFO,     EBML_NONE },
-    { MATROSKA_ID_CUES,     EBML_NONE },
-    { MATROSKA_ID_TAGS,     EBML_NONE },
-    { MATROSKA_ID_SEEKHEAD, EBML_NONE },
-    { 0 }
-};
-
-static const EbmlSyntax matroska_cluster_incremental_parsing[] = {
+static const EbmlSyntax matroska_cluster_parsing[] = {
     { MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0,                     offsetof(MatroskaCluster, timecode) },
     { MATROSKA_ID_BLOCKGROUP,      EBML_NEST, sizeof(MatroskaBlock), offsetof(MatroskaCluster, blocks), { .n = matroska_blockgroup } },
     { MATROSKA_ID_SIMPLEBLOCK,     EBML_PASS, sizeof(MatroskaBlock), offsetof(MatroskaCluster, blocks), { .n = matroska_blockgroup } },
@@ -741,7 +720,7 @@  static const EbmlSyntax matroska_cluster_incremental_parsing[] = {
     { 0 }
 };
 
-static const EbmlSyntax matroska_cluster_incremental[] = {
+static const EbmlSyntax matroska_cluster[] = {
     { MATROSKA_ID_CLUSTERTIMECODE, EBML_UINT, 0, offsetof(MatroskaCluster, timecode) },
     { MATROSKA_ID_BLOCKGROUP,      EBML_STOP },
     { MATROSKA_ID_SIMPLEBLOCK,     EBML_STOP },
@@ -750,8 +729,8 @@  static const EbmlSyntax matroska_cluster_incremental[] = {
     { 0 }
 };
 
-static const EbmlSyntax matroska_clusters_incremental[] = {
-    { MATROSKA_ID_CLUSTER,  EBML_NEST, 0, 0, { .n = matroska_cluster_incremental } },
+static const EbmlSyntax matroska_clusters[] = {
+    { MATROSKA_ID_CLUSTER,  EBML_NEST, 0, 0, { .n = matroska_cluster } },
     { MATROSKA_ID_INFO,     EBML_NONE },
     { MATROSKA_ID_CUES,     EBML_NONE },
     { MATROSKA_ID_TAGS,     EBML_NONE },
@@ -2648,8 +2627,6 @@  static int matroska_parse_tracks(AVFormatContext *s)
             }
         } else if (track->type == MATROSKA_TRACK_TYPE_SUBTITLE) {
             st->codecpar->codec_type = AVMEDIA_TYPE_SUBTITLE;
-            if (st->codecpar->codec_id == AV_CODEC_ID_ASS)
-                matroska->contains_ssa = 1;
         }
     }
 
@@ -3520,19 +3497,19 @@  end:
     return res;
 }
 
-static int matroska_parse_cluster_incremental(MatroskaDemuxContext *matroska)
+static int matroska_parse_cluster(MatroskaDemuxContext *matroska)
 {
     EbmlList *blocks_list;
     MatroskaBlock *blocks;
     int i, res;
     res = ebml_parse(matroska,
-                     matroska_cluster_incremental_parsing,
+                     matroska_cluster_parsing,
                      &matroska->current_cluster);
     if (res == 1) {
         /* New Cluster */
         if (matroska->current_cluster_pos)
             ebml_level_end(matroska);
-        ebml_free(matroska_cluster, &matroska->current_cluster);
+        ebml_free(matroska_cluster_parsing, &matroska->current_cluster);
         memset(&matroska->current_cluster, 0, sizeof(MatroskaCluster));
         matroska->current_cluster_num_blocks = 0;
         matroska->current_cluster_pos        = avio_tell(matroska->ctx->pb);
@@ -3540,12 +3517,12 @@  static int matroska_parse_cluster_incremental(MatroskaDemuxContext *matroska)
         if (matroska->current_id)
             matroska->current_cluster_pos -= 4;
         res = ebml_parse(matroska,
-                         matroska_clusters_incremental,
+                         matroska_clusters,
                          &matroska->current_cluster);
         /* Try parsing the block again. */
         if (res == 1)
             res = ebml_parse(matroska,
-                             matroska_cluster_incremental_parsing,
+                             matroska_cluster_parsing,
                              &matroska->current_cluster);
     }
 
@@ -3576,35 +3553,6 @@  static int matroska_parse_cluster_incremental(MatroskaDemuxContext *matroska)
     return res;
 }
 
-static int matroska_parse_cluster(MatroskaDemuxContext *matroska)
-{
-    MatroskaCluster cluster = { 0 };
-    EbmlList *blocks_list;
-    MatroskaBlock *blocks;
-    int i, res;
-    int64_t pos;
-
-    if (!matroska->contains_ssa)
-        return matroska_parse_cluster_incremental(matroska);
-    pos = avio_tell(matroska->ctx->pb);
-    if (matroska->current_id)
-        pos -= 4;  /* sizeof the ID which was already read */
-    res         = ebml_parse(matroska, matroska_clusters, &cluster);
-    blocks_list = &cluster.blocks;
-    blocks      = blocks_list->elem;
-    for (i = 0; i < blocks_list->nb_elem; i++)
-        if (blocks[i].bin.size > 0 && blocks[i].bin.data) {
-            int is_keyframe = blocks[i].non_simple ? blocks[i].reference == INT64_MIN : -1;
-            res = matroska_parse_block(matroska, blocks[i].bin.buf, blocks[i].bin.data,
-                                       blocks[i].bin.size, blocks[i].bin.pos,
-                                       cluster.timecode, blocks[i].duration,
-                                       is_keyframe, NULL, 0, 0, pos,
-                                       blocks[i].discard_padding);
-        }
-    ebml_free(matroska_cluster, &cluster);
-    return res;
-}
-
 static int matroska_read_packet(AVFormatContext *s, AVPacket *pkt)
 {
     MatroskaDemuxContext *matroska = s->priv_data;
@@ -3699,7 +3647,7 @@  static int matroska_read_close(AVFormatContext *s)
     for (n = 0; n < matroska->tracks.nb_elem; n++)
         if (tracks[n].type == MATROSKA_TRACK_TYPE_AUDIO)
             av_freep(&tracks[n].audio.buf);
-    ebml_free(matroska_cluster, &matroska->current_cluster);
+    ebml_free(matroska_cluster_parsing, &matroska->current_cluster);
     ebml_free(matroska_segment, matroska);
 
     return 0;