Use existing FrameTransformerInterface?

[alvestrand]

We have a bunch of classes defined for transforming stuff already - api/frame_transformer_interface.h gives details.

If the SFrame transform can be implemented as a FrameTransformerInterface (with suitable extensions where necessary), I think we'll have a much easier time supporting the scaffolding going forward.

[alvestrand]

The biggest difference is probably that we'll have to define a RegisterTrasnformedFrameCallback that takes a sequence of frames, not just one, since we need to allow for the transformer breaking up a single frame in multiple packets for SPacket.

[k-wasniowski]

Hey @alvestrand
Thank you, we will investigate this approach. One thing which comes to my mind is how we will let sender/receiver know that we are actually registering sframe transformer with generic SetFrameTransformer call. It has to trigger negotiation needed and update transceiver state.

Would it make sense to keep SetSFrameTransformer so that it would know that it actually should perform mentioned above actions - just update it to use FrameTransformerInterface

[alvestrand]

I think that's a confusion of levels.
SetFrameTransformer is a low level API - it does not call up to higher level concepts like NegotiationNeeded.
The function that calls SetFrameTransformer(sframe transform) in response to a Javascript action needs to do all those other actions too - it is not a C++ FrameTransformerInterface level concept.

[k-wasniowski]

@alvestrand In that case, stuff like injecting sframe attribute to sdp would need to be done on the chromium blink level, right?

[alvestrand]

Rereading SetFrameTransformer, and figuring out that RtpSenderInterface and RtpReceiverInterface export FrameTransformerHost directly.....

I would perhaps expect that on a FrameTransformerInterface object, one property of the FrameTransformerInterface object would be "SdpModifications", a function returning "a=sframe" (or perhaps an enum with the "isSframe" value, to narrow the injection surface). Just like sender.AddTrack, this would be able to trigger ONN.

[k-wasniowski]

Sounds reasonable, need to figure out how it will fit into the blink which doesnt call SetFrameTransformer directly with newly created transformer but rather pass it though delegate which is being created on the initialization here:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/peerconnection/rtc_rtp_sender_impl.cc;l=180-211

[k-wasniowski]

@alvestrand Could you point which sender.AddTrack you had in mind? I have not found any places in which sender has a AddTrack, as it rather on the Peer Connection level. But maybe i'm missing something.
We have played around a little bit and came up with the idea that we could extend FrameTransformerInterface with an ability to register for an SDP modification events. Something like:

class FrameTransformerSdpAttributeObserver : public RefCountInterface {
 public:
  // Or maybe instead of list of attributes, we should have predefined list of available modifications?
  virtual void OnSdpAttributesChanged(
      const std::map<std::string, std::string>& attributes) = 0;

 protected:
  ~FrameTransformerSdpAttributeObserver() override = default;
};

And extend the interface:

class FrameTransformerInterface : public RefCountInterface {
 public:
  // SDP attribute injection support for transformers (e.g., encryption params)
  // Register an observer to be notified when custom SDP attributes change.
  virtual void RegisterSdpAttributeObserver(
      scoped_refptr<FrameTransformerSdpAttributeObserver> observer) {}
  
  // Unregister the previously registered SDP attribute observer.
  virtual void UnregisterSdpAttributeObserver() {}

 protected:
  ~FrameTransformerInterface() override = default;
};

This way RtpSenderBase could inherit from the observer and listen for incoming modification requests...

It's mainly due to the fact that on blink level it doesnt really recreate a transformer which get's injected into libwebrtc, but rather creates a delegate once, and operate on the higher levels.

[k-wasniowski]

Hey @alvestrand In the case, if we would like to stick to have only one setter method for frame transformer and skip the set packet transformer, how should algorithm look like on the for example RTPSenderVideo level. Right now, it looks like:

1. Receive a encoded video frame
2. Pass encoded frame to frame transformer
3. Transformed frame received at RTPSenderVideo::SendVideo method
4. SendVideo method packetize frames according to provided video type
5. RTP packets send to the network layer.

RTPSenderVideo through the frame transformer delegate will send a frame to the delegate to yhe transformer, and receice a transformed frame with the registered callback.

It works perfectly with the per-frame transformation, as frame transformer will get and return decrypted frame, and the packetizer will be able to detect that media type have changed from some video codec lets say vp8 to sframe, and select appropiate packetizer.

1. Receive a encoded video frame
2. Pass encoded frame to frame transformer
3. Transformer encrypts a frame payload
4. Transformer sets a video type to sframe
5. Transformed frame received at RTPSenderVideo::SendVideo method
6. SendVideo method packetize frames (with sframe packetizer)
7. RTP packets send to the network layer.

For the packet level encryption we need to have a flow in which transformer will be called after a media specific packetizer like:

1. Receive a encoded video frame
2. Pass encoded frame to frame transformer
3. Transformed frame received at RTPSenderVideo::SendVideo method
4. SendVideo method packetize frames according to provided video type
5. Push all packets to the frame transformer delegate (sframe)
6. Transformer encrtypts and updates packets with sframe payload header
7. Transformed packets gets returned back to the RTPSenderVideo through the callback
8. RTP packets send to the network layer.

In this scenario we would need to register a frame transformer callback method in two places which is not possible. Or we would need to extend Transformer interface to be able to distinhuish to which sink it should send a frame/packet but it doesnt seem right.

[k-wasniowski]

Hmmm or maybe it should be responsibility of the frame transformer delegate to track if provided unit is a frame or packet, and push it to appropriate sink in thd RTPSenderVideo...