[pbs-devel] [RFC v2 pxar 04/36] decoder: add optional payload input stream

[Fabian Grünbichler]

On March 11, 2024 3:05 pm, Christian Ebner wrote:
>> On 11.03.2024 14:21 CET Fabian Grünbichler <f.gruenbichler at proxmox.com> wrote:
>> 
>>  
>> 
>> same question as for the encoder - do we want to prevent misuse here and
>> check/ensure that no payload_input has already been set before?
>>
> 
> Also for the decoder I will opt for the suggestion by Dietmar and lock this
> down for possible misuse by adding the optional parameter to the `new` method
> instead.

ack

>> style: these two could be combined into
>> 
>> if let Some(payload_input) = self.payload_input.as_mut() {
>>     if seq_read_position(payload_input).await.transpose()?.is_none() {
>>        // Skip payload padding for injected chunks in sync decoder
>>        let to_skip = payload_ref.offset - self.payload_consumed;
>>        self.skip_payload(to_skip).await?;
>>     }
>> }
>>
> 
> Good, this will come in handy as Dietmar also suggested to add a header for
> all the payloads stored in the separated payload archive. I plan on using the
> existing PXAR_PAYLOAD header for this, also encoding the payload size. I will
> therefore add a check for the header here and compare the sizes as found in
> the payload input and metadata archives.

out of intereset - what do we need the header for? or is it just to make
the resulting payload archive more debuggable?

>> nit: this could also share the "skip" part with `skip_entry`, and take the
>> input and length as parameter?
> 
> I opted against that, since `skip_entry` operates only on the metadata
> archive, the `skip_payload` however on the `payload_input`. I could however
> factor this out into a function which takes the input as parameter, making
> thereby sure to have the correct input to skip bytes.

yes, that's what I meant - they are identical except for where the input
and length come from.

>> nit: casting to usize at the start would make the code easier to parse
>> IMHO
> 
> Okay, I can send that as independent patch already I guess.