cinecert / asdcplib

AS-DCP and AS-02 File Access Library
Other
70 stars 54 forks source link

ReadFreame method in as_02_IAB.cpp needs re-work #61

Open kblinova opened 4 years ago

kblinova commented 4 years ago

There are two problems with the implementation of this method:

  1. The method relies on the encoding of the IAFrame to find the frame size; IAFrame is described in SMPTE ST 2098-2:2019 and can change independently of SMPTE ST 2067-201:2019. There is no need to tie the two together.

  2. The method uses small read/seek calls that are inefficient and problematic in future cloud deployments. The method should ultimately make one call to read (with possibly no seek if the frames are read sequentially) to ingest the whole frame.

The size of the frame should be determined by looking at the position of the next frame in the IndexTable. The index table is already loaded into memory and will not require any additional calls to the underlying file.

palemieux commented 4 years ago

The size of the frame should be determined by looking at the position of the next frame in the IndexTable.

@kblinova What about the last frame?

kblinova commented 4 years ago

The end of the clip element.

palemieux commented 4 years ago

Couple of further thoughts.

The method uses small read/seek calls that are inefficient and problematic in future cloud deployments. The method should ultimately make one call to read (with possibly no seek if the frames are read sequentially) to ingest the whole frame.

There are two separate issues:

  1. one seek per frame read. this is primarily due to the function that reads one frame at a time and with random access. Two potential solutions: a. define a streaming read function that reads continuously after an initial seek b, keep a memory of the previous frame index to avoid a seek unless needed

IAFrame is described in SMPTE ST 2098-2:2019 and can change independently of SMPTE ST 2067-201:2019. There is no need to tie the two together.

ST 2067-201 defines an IA Track File, which, by definition, contains a sequence of IA Frames. Can you clarify what you have in mind.

kblinova commented 4 years ago

There is no goal to eliminate at least one seek and at least one read per frame. It is required due to the nature of the random access of frames.

What needs to be avoided is having the small reads to get the size of the frame from the IAFrame encoding. When this format is taken to the could, these fread calls would have to be translated to API calls to access the cloud resources so minimizing the number of calls could be a useful strategy.

The index table has all the information that is needed to access the frames, so there is no need to use the fact that the essence is IAFrame in order to decode it. The MXF wrapper should be essence-agnostic and is designed to have all the information to carry any essence without relying on the information about how the content is encoded.

palemieux commented 4 years ago

The index table has all the information that is needed to access the frames,

However the index table does not contain sufficient information to determine the size of a frame, e.g. does not account for KLV Fill items.

What needs to be avoided is having the small reads to get the size of the frame from the IAFrame encoding.

Doesn't the cloud I/O library buffer reads?