This looks very promising! I suggest to wait until the base patch is merged to actually review this. A couple of questions in the meantime:

• What happens when copy-pasting across different points in the timeline?
• What happens to duplicated strips when all channels are full?

In D12513#326679, @Francesco Siddi (fsiddi) wrote:

• What happens when copy-pasting across different points in the timeline?

As it is now, is it pasting at the same position as it is copied from, but in the next free channel. It could paste at the playhead position, but my guess is that it'll be more useful to paste at the same position in this specific scenario.

• What happens to duplicated strips when all channels are full?

The same thing as normal paste, with several strips, it'll fail, but that's related to the paste operator and unrelated to this patch:

I think it is good idea to have operators like these accessible in preview. After quick discussion, some problems will appear:

Transform operator uses same logic as VSE render pipeline to get list of strips to be rendered. This seems reasonable to me as you expect to manipulate only images you see. So movie with 3 effects attached, would result in 3 selectable strips - effects, because you can't see original movie, and if you transform it, even unselected images would move. I assume other operators should follow this logic.

Respecting the above rule this with keyboard shortcuts is easy - operator can detect in what region it was invoked and choose on what it will operate.

However using menu items in combined timeline+preview mode would be problematic, because operator has no idea on what it should operate. Not sure how this could be resolved easily without duplicating menu items. Perhaps there could be some flag indicating where (last) selection was made, but not sure if this would result in predictable behavior. Probably not, because this could interfere with normal manipulation in timeline. Alternative could be to keep operators in combined view to work only on all selected strips.

In D12513#328288, @Richard Antalik (ISS) wrote:

Transform operator uses same logic as VSE render pipeline to get list of strips to be rendered. This seems reasonable to me as you expect to manipulate only images you see. So movie with 3 effects attached, would result in 3 selectable strips - effects, because you can't see original movie, and if you transform it, even unselected images would move. I assume other operators should follow this logic.

I don't understand how this relates to this patch and what to do about it?

However using menu items in combined timeline+preview mode would be problematic, because operator has no idea on what it should operate. Not sure how this could be resolved easily without duplicating menu items. Perhaps there could be some flag indicating where (last) selection was made, but not sure if this would result in predictable behavior. Probably not, because this could interfere with normal manipulation in timeline. Alternative could be to keep operators in combined view to work only on all selected strips.

Above there is a gif showing the preview select functions exposed as a select-sub-menu when in sequencer&preview.

Just noticed, that this patch adds macros to transform some operators as different with specific purpose. I don't think this is great idea. I would propose something like this as example:

diff --git a/source/blender/editors/space_sequencer/sequencer_select.c b/source/blender/editors/space_sequencer/sequencer_select.c
index aa6599a7c53..2b115d2717e 100644
--- a/source/blender/editors/space_sequencer/sequencer_select.c
+++ b/source/blender/editors/space_sequencer/sequencer_select.c
@@ -406,17 +406,32 @@ static void sequencer_select_do_updates(bContext *C, Scene *scene)
 /** \name (De)select All Operator
  * \{ */

+static SeqCollection *seq_all_strips_from_context(bContext *C)
+{
+  Scene *scene = CTX_data_scene(C);
+  Editing *ed = SEQ_editing_get(scene);
+  ARegion *region = CTX_wm_region(C);
+  SeqCollection *strips;
+  if (region->regiontype == RGN_TYPE_PREVIEW) {
+    strips = SEQ_query_rendered_strips(SEQ_active_seqbase_get(ed), CFRA, 0);
+  }
+  else {
+    strips = SEQ_query_all_strips(SEQ_active_seqbase_get(ed));
+  }
+  return strips;
+}
+
 static int sequencer_de_select_all_exec(bContext *C, wmOperator *op)
 {
   int action = RNA_enum_get(op->ptr, "action");

   Scene *scene = CTX_data_scene(C);
-  Editing *ed = SEQ_editing_get(scene);
-  Sequence *seq;
+  SeqCollection *strips = seq_all_strips_from_context(C);

   if (action == SEL_TOGGLE) {
     action = SEL_SELECT;
-    for (seq = ed->seqbasep->first; seq; seq = seq->next) {
+    Sequence *seq;
+    SEQ_ITERATOR_FOREACH (seq, strips) {
       if (seq->flag & SEQ_ALLSEL) {
         action = SEL_DESELECT;
         break;
@@ -424,7 +439,8 @@ static int sequencer_de_select_all_exec(bContext *C, wmOperator *op)
     }
   }

-  for (seq = ed->seqbasep->first; seq; seq = seq->next) {
+  Sequence *seq;
+  SEQ_ITERATOR_FOREACH (seq, strips) {
     switch (action) {
       case SEL_SELECT:
         seq->flag &= ~(SEQ_LEFTSEL + SEQ_RIGHTSEL);

Now you can use seq_all_strips_from_context() in all operator that iterated over whole ed->seqbasep and they would automatically operate either in preview or timeline context without need for any macro or custom property forcing it to operate in either context. That said seq_all_strips_from_timeline_or_preview() would be probably better name for the function.

In D12513#328309, @Peter Fog (tintwotin) wrote:

In D12513#328288, @Richard Antalik (ISS) wrote:

Transform operator uses same logic as VSE render pipeline to get list of strips to be rendered. This seems reasonable to me as you expect to manipulate only images you see. So movie with 3 effects attached, would result in 3 selectable strips - effects, because you can't see original movie, and if you transform it, even unselected images would move. I assume other operators should follow this logic.

I don't understand how this relates to this patch and what to do about it?

If selection is to be handled differently in preview, then all operators should probably follow same logic.

However using menu items in combined timeline+preview mode would be problematic, because operator has no idea on what it should operate. Not sure how this could be resolved easily without duplicating menu items. Perhaps there could be some flag indicating where (last) selection was made, but not sure if this would result in predictable behavior. Probably not, because this could interfere with normal manipulation in timeline. Alternative could be to keep operators in combined view to work only on all selected strips.

Above there is a gif showing the preview select functions exposed as a select-sub-menu when in sequencer&preview.

These menu items are effectively duplicate, so if there are not too many of them and they apply to image "properties" only, I guess that is also viable alternative but I am not aware of this approach used elsewhere.

@Richard Antalik (ISS) This breaks selection in the Preview:

In D12513#328315, @Richard Antalik (ISS) wrote:

Just noticed, that this patch adds macros to transform some operators as different with specific purpose. I don't think this is great idea. I would propose something like this as example:

diff --git a/source/blender/editors/space_sequencer/sequencer_select.c b/source/blender/editors/space_sequencer/sequencer_select.c
index aa6599a7c53..2b115d2717e 100644
--- a/source/blender/editors/space_sequencer/sequencer_select.c
+++ b/source/blender/editors/space_sequencer/sequencer_select.c
@@ -406,17 +406,32 @@ static void sequencer_select_do_updates(bContext *C, Scene *scene)
 /** \name (De)select All Operator
  * \{ */

+static SeqCollection *seq_all_strips_from_context(bContext *C)
+{
+  Scene *scene = CTX_data_scene(C);
+  Editing *ed = SEQ_editing_get(scene);
+  ARegion *region = CTX_wm_region(C);
+  SeqCollection *strips;
+  if (region->regiontype == RGN_TYPE_PREVIEW) {
+    strips = SEQ_query_rendered_strips(SEQ_active_seqbase_get(ed), CFRA, 0);
+  }
+  else {
+    strips = SEQ_query_all_strips(SEQ_active_seqbase_get(ed));
+  }
+  return strips;
+}
+
 static int sequencer_de_select_all_exec(bContext *C, wmOperator *op)
 {
   int action = RNA_enum_get(op->ptr, "action");

   Scene *scene = CTX_data_scene(C);
-  Editing *ed = SEQ_editing_get(scene);
-  Sequence *seq;
+  SeqCollection *strips = seq_all_strips_from_context(C);

   if (action == SEL_TOGGLE) {
     action = SEL_SELECT;
-    for (seq = ed->seqbasep->first; seq; seq = seq->next) {
+    Sequence *seq;
+    SEQ_ITERATOR_FOREACH (seq, strips) {
       if (seq->flag & SEQ_ALLSEL) {
         action = SEL_DESELECT;
         break;
@@ -424,7 +439,8 @@ static int sequencer_de_select_all_exec(bContext *C, wmOperator *op)
     }
   }

-  for (seq = ed->seqbasep->first; seq; seq = seq->next) {
+  Sequence *seq;
+  SEQ_ITERATOR_FOREACH (seq, strips) {
     switch (action) {
       case SEL_SELECT:
         seq->flag &= ~(SEQ_LEFTSEL + SEQ_RIGHTSEL);

Now you can use seq_all_strips_from_context() in all operator that iterated over whole ed->seqbasep and they would automatically operate either in preview or timeline context without need for any macro or custom property forcing it to operate in either context. That said seq_all_strips_from_timeline_or_preview() would be probably better name for the function.

I don't think it is good idea to have 2 menu items for 1 function, but in 2 different contexts. For this particular view type, solution proposed in second paragraph in https://developer.blender.org/T92584#1251401 would be more optimal I think.

In any case we shouldn't fix features by replacing them witn new different features, if it's broken in current state it should be reverted or disabled for this view type.

In D12513#352641, @Richard Antalik (ISS) wrote:

I don't think it is good idea to have 2 menu items for 1 function, but in 2 different contexts. For this particular view type, solution proposed in second paragraph in https://developer.blender.org/T92584#1251401 would be more optimal I think.

It is not clear to me what you mean in that paragraph? I don't think removing features in Sequencer/Preview is a better alternative than exposing similar named but ultimately different functions in different top menus, if that's what you mean?
This is much better than not having those functions:

Currently, in Image(preview), basic functions like copy/paste/duplicate etc. are not even implemented as shortcuts in the Preview, nor UI(this patch):

Also, currently, the Image(preview) context menu only holds a Rename operator. This patch:

Anyway, this macro solution/"double exposure" solution is imo to be preferred as a placeholder, over the current broken implementation, until it is decided on the major UI changes you and Campbell are aiming at. What does @Francesco Siddi (fsiddi) think?

In D12513#352641, @Richard Antalik (ISS) wrote:

In any case we shouldn't fix features by replacing them with new different features, if it's broken in the current state it should be reverted or disabled for this view type.

The included features in this patch were already there in my original patch from the 16. of september. As mentioned copy, paste, duplicate, selection y shortcut etc. are basic functions, which users would expect to work in the Preview.

If it needs to be cut in stone, in Sequencer/Preview all functions relating to the Preview could be collected in a top-level menu called Preview: